Error when displaying log from a previous VS Code run after restarting

[edoardopirovano]

Describe the bug
After a restart, items in the Query History view preserved from previous runs still have a Show Query Log button when right-clicked. However, the query log is gone as it is stored in temporary storage that is cleared on restart, so clicking the button gives an error.

Version
Latest main of both CodeQL and VS Code.

To reproduce

1. Run a query.
2. Restart VS Code.
3. Right click the Query History item for the run and select Show Query Log.

Expected behavior

Either we don't show Show Query Log any more, or we place the log somewhere where it is preserved across runs.

[aeisenberg]

Thanks for reporting. I think the best thing to do would be to ensure the log file is saved to the query history location in the global storage. The problem manifests here:

vscode-codeql/extensions/ql-vscode/src/queryserver-client.ts

Lines 211 to 216 in cdb9506

	const baseLocation = this.logger.getBaseLocation();
	if (baseLocation && this.activeQueryName) {
	res.logFileLocation = path.join(baseLocation, this.activeQueryName);
	}
	this.evaluationResultCallbacks[res.runId](res);
	}

Unfortunately, the logic is a bit twisted and we don't have the query history location available at the moment when we start writing the logs. It will require a little bit of reworking to get there.

[edoardopirovano]

Indeed. I noticed this because I replicated the existing logic for some new options to display per-query structured logs and encountered this there too.

I started looking into fixing this but I was actually going the opposite way and trying to figure out when a query comes from a previous run to hide the option. That isn't too easy to do either, so your solution is probably better.

[aeisenberg]

Let me take this over since this is related to work I've just done. I'll focus on the existing log and I'll make sure that whatever I do will be easily extendable to the structured log.

[aeisenberg]

Going through this now. The good news is that a lot of this logic can be significantly simplified since log files no longer need to be deleted when the workspace closes because they are deleted when the rest of the results are deleted.

The bad (or complicating) news is that there is a preference codeQL.runningQueries.customLogDirectory that no longer makes sense to support since its reason for being around was to allow users to retain logs across restarts. We could retain this behaviour for backwards compatibility. Doing so will add much of the complexity back. Or we could remove the handling of the setting and if the extension detects the setting, we can issue a pop-up warning describing the new behaviour.

I won't make any changes until we can discuss this some more.

[edoardopirovano]

Or we could remove the handling of the setting and if the extension detects the setting, we can issue a pop-up warning describing the new behaviour.

This sounds like a sensible option to me. The only reason that was added was because users wanted to retain logs after closing VS Code. If we're now in a position to store logs in a place that isn't deleted across restarts, there isn't really any need to keep that option.

I'll wait to see what others think, though. In particular @shati-patel might have some thoughts as she worked on that feature if I remember rightly 🙂

[shati-patel]

Or we could remove the handling of the setting and if the extension detects the setting, we can issue a pop-up warning describing the new behaviour.

This sounds like a sensible option to me. The only reason that was added was because users wanted to retain logs after closing VS Code. If we're now in a position to store logs in a place that isn't deleted across restarts, there isn't really any need to keep that option.

I'll wait to see what others think, though. In particular @shati-patel might have some thoughts as she worked on that feature if I remember rightly 🙂

Your reasoning sounds sensible to me! The custom log directory was in response to #820, which mentions "persisting logs across restarts" as the main use case.

I agree it's fine to delete the codeQL.runningQueries.customLogDirectory setting now 😸

[edoardopirovano]

Nice, thanks for digging up that issue. @tausbn as the original requester of that option, would you have any objections to its removal if we were now persisting logs across restarts until a query is deleted from the Query History?

[aeisenberg]

Here is how I am suggesting we handle the feature removal:

1. For now, retain the codeQL.runningQueries.customLogDirectory setting, but change the description to say that it is deprecated, no longer works and will be removed.
2. Any time we run a query and we detect that there is a customLogDirectory set, we can issue a warning saying that the query log will actually be saved elsewhere.
3. At some point in the future after a few releases have gone by, we can completely remove the setting and the warning message.

[tausbn]

Nice, thanks for digging up that issue. @tausbn as the original requester of that option, would you have any objections to its removal if we were now persisting logs across restarts until a query is deleted from the Query History?

As long as said persistence works across restarts, including restarting a Codespaces instance, then I would be happy. Note that the current setup cannot even preserve the CLI across restarts/reconnects when Codespaces are involved. (I currently have to reinstall the CLI half a dozen times per day.) The reason the custom log directory worked for this purpose is that I could set it to a subdirectory of my codeql checkout, and Codespaces takes great care not to delete files stored there.

Though, to be quite sure, I would need to understand what "[...] when the rest of the results are deleted." means. When exactly does this deletion occur?

Also, due to their enormous size, I never actually open these logs in VSCode, but rather open them in the terminal using less. This is easy and convenient when I can control the output directory myself. If there could be an option to copy the log file path to the clipboard, that would be sufficient.

[aeisenberg]

When exactly does this deletion occur?

Results are kept around for 30 days by default and this value can be changed in your settings. These results will be retained in the query history view for your workspace. Also, manually removing results from the query history view also deletes them from disk.

If there could be an option to copy the log file path to the clipboard, that would be sufficient.

Thanks for the idea. I will make sure it is easy to open log files outside of vscode.