Conversation
6e3fa64 to
a387486
Compare
c0b46e3 to
0eaec4f
Compare
src/codeql.ts
Outdated
| }, | ||
| }).exec(); | ||
|
|
||
| return JSON.parse(output); |
There was a problem hiding this comment.
Catch malformed json errors and rethrow with a meaningful message.
| return JSON.parse(output); | |
| try { | |
| return JSON.parse(output); | |
| } catch (e) { | |
| throw new Error("Something meaningful.); | |
| } |
There was a problem hiding this comment.
Wouldn't that suppress the likely interesting error message of e ? In Java I'd write new Error("Unexpected output from codeql resolve languages", e) . What would be the equivalent in Javascript?
There was a problem hiding this comment.
I think just construct the message as a string. So like new Error("Unexpected output from codeql resolve languages: " + e.message) and maybe some type checks to make sure that e.message exists.
There was a problem hiding this comment.
I opted to just write Unexpected output from codeql resolve languages: ${e} . This should include the original error's type and side-steps the problem of a potentially empty message string.
aeisenberg
left a comment
There was a problem hiding this comment.
We're populating the changelog now. Since this is a public-facing feature, could you add an entry. And then it's good to merge. Thanks!
There shouldn't be an entry. This is just for internal tests. CodeQL for Ruby is not available yet. |
Merge / deployment checklist