Rework invocation error card and render more types of errors #6407
Conversation
bduffany
force-pushed
the
error-card-rework
branch
from
cec313c
to
de0ea36
Compare
bduffany
force-pushed
the
error-card-rework
branch
2 times, most recently
from
259eaa6
to
782aaa8
Compare
bduffany
force-pushed
the
error-card-rework
branch
from
782aaa8
to
9c0913b
Compare
|
|
||
| componentDidUpdate(prevProps: Props): void { | ||
| const model = this.getModel(this.props); | ||
| if (!modelsEqual(this.getModel(this.props), this.getModel(prevProps))) { |
There was a problem hiding this comment.
| if (!modelsEqual(this.getModel(this.props), this.getModel(prevProps))) { | |
| if (!modelsEqual(model, this.getModel(prevProps))) { |
| prettyPrintedStdErr() { | ||
| if (!this.state.stdErr) { | ||
| return ""; | ||
| getModel(props = this.props): CardModel { |
There was a problem hiding this comment.
small nit, i feel like defaulting is kinda error-prone here, and given that you only use it this way in componentDidMount, you might as well make this a standalone function
| // If there was a failed action with a non-zero exit code, use that as the title. | ||
| for (const error of model.errors) { | ||
| if ((error.action?.exitCode ?? 0) !== 0) { | ||
| return `${this.props.model.failedAction?.action?.type} action failed with exit code ${this.props.model.failedAction?.action?.exitCode}`; |
There was a problem hiding this comment.
any better value to put here for type ? i guess this is fine, it just drives me nuts that nullishness isn't explicitly linked between the two.
There was a problem hiding this comment.
made it default to "Build" so it'll render as "Build action" if it's missing 🤡
| // If the finished event exists and there's a non-zero exit code, use that as the card title. | ||
| for (const error of model.errors) { | ||
| if ((error.finished?.exitCode ?? 0) !== 0) { | ||
| return exitCode(error.finished?.exitCode?.name ?? ""); |
There was a problem hiding this comment.
i think all of the wildcards on the return here are not needed and name is defined, right?
There was a problem hiding this comment.
I tried this originally but I think the compiler isn't smart enough to narrow the type in this case - it doesn't seem to draw the connection between the ?? 0 and the !== 0
Before:
After:
Before:
After:
Related issues: Fixes #6404