Do not warn users if a workflow cannot be read - #370
simon-engledew merged 6 commits into
Conversation
Sorry, something went wrong.
I don't think this will send a LintFailed message status report. If getWorkflow returns undefined then it does nothing. You want to change getWorkflowErrors to do if (workflow === undefined) {
return [WorkflowErrors.LintFailed];
}Ideally we'd also have different kinds of LintFailed so we can tell between errors in the workflow and failing to read the workflow. |
Sorry, something went wrong.
|
I agree this will silence the error message. Up to you if you want to also upload the failure message in the status report or just leave it. |
Sorry, something went wrong.
Ugh, sorry - quite right. I should have actually checked this. 🤦 🙇 I've had a bit of a refactor and tweaked the way we report errors to include the exception string if something failed during linting |
Sorry, something went wrong.
robertbrignull
left a comment
There was a problem hiding this comment.
One suggestion about a try-catch but LGTM regardless
Sorry, something went wrong.
| } | ||
| } catch (e) { | ||
| return [WorkflowErrors.LintFailed]; | ||
| return `error: getWorkflow() failed: ${e.toString()}`; |
There was a problem hiding this comment.
To avoid this try-catch covering more than necessary you could do
let workflow: Workflow;
try {
workflow = await getWorkflow();
} catch (e) {
return `error: getWorkflow() failed: ${e.toString()}`;
}and that way you can assign a variable to a non-nullable type but have the try-catch be more contained so it's easier to read and you won't unexpectedly catch something you didn't mean to.
Sorry, something went wrong.
There was a problem hiding this comment.
Oh, it's nice that the type checker is sophisticated enough to realise that the variable is not nullable!
I've used that technique to add more fine grained handling over the whole function, thanks! 👍
Sorry, something went wrong.
This will still send a LintFailed message status report.
Hide these warnings:
This will still send a LintFailed message status report.