Improve error messages when no languages or no queries are specified #131
Conversation
| throw new Error("Did not detect any languages to analyze. " + | ||
| "Please update input in workflow or check that GitHub detects the correct languages in your repository."); | ||
| } | ||
|
|
There was a problem hiding this comment.
Sorry if this is a silly question @robertbrignull but will the error appear more prominently (in the Check failure summary, rather than only in the detailed logs) if thrown from here rather than in the calling method as it was before these changes? Or do we also need to change how we throw the error?
There was a problem hiding this comment.
Where the error is thrown from doesn't make a difference to how it is presented.
I'm going to do a test of this to make sure it works and looks presentable before this is merged.
There was a problem hiding this comment.
👍 I'd love a link to see what it looks like too!
chrisgavin
left a comment
chrisgavin
left a comment
There was a problem hiding this comment.
Great! Hopefully this will reduce confusion, particularly as more people start copying their existing CodeQL workflows to new projects that don't yet have any Linguist detected languages.
src/config-utils.ts
Outdated
| // it is a user configuration error. | ||
| for (const language of languages) { | ||
| if (queries[language].length === 0) { | ||
| throw new Error(`Did not detect any queries to analyze for ${language}. ` + |
There was a problem hiding this comment.
| throw new Error(`Did not detect any queries to analyze for ${language}. ` + | |
| throw new Error(`Did not detect any queries to run for ${language}. ` + |
robertbrignull
force-pushed
the
languages_error
branch
from
d9347d3 to
44c88fd
Compare
|
I've done some test runs:
During testing I discovered a bug with checking the set of queries so I've fixed that. I also ran into a problem of not being able to see the error stacktrace. I've opened #136 to hopefully address this. |
This should help address some unhelpful error messages being seen, for example at https://github.com/octokit/auth-unauthenticated.js/runs/943539485?check_suite_focus=true
The error there is that we didn't detect any languages to analyze, because ther weren't any specified in the workflow file, and linguist didn't detect any because I believe there wasn't any typescript code on the default branch at that time. The branch where the analysis ran was introducing the first piece of analyzable code.
So although the above is perhaps the true cause of the failure, the error message made this very unclear. In the case where there isn't a config file we weren't checking that the set of languages was non-empty. This is a bug and so I've copied the check from the config file case over into a shared method so it also runs when there's no config file.
Another potential configuration setup that would cause the same bad error message is if no queries are specified for a particular language. That didn't happen here but I think it's likely it'll happen at some point. I also add a check for this case so we'll report a better error message and report during config parsing instead of after extraction.
Merge / deployment checklist