PipelineDetails page tests #380
Conversation
| pipeline = await Apis.pipelineServiceApi.getPipeline(pipelineId); | ||
| } catch (err) { | ||
| await this.showPageError('Cannot retrieve pipeline details.', err); | ||
| logger.error('Cannot retrieve pipeline details.', err); |
There was a problem hiding this comment.
Not needed in this PR, but maybe we should create a util function that does these two lines with the two inputs (message and error) since it's always the same.
We can then add a comment explaining why it's useful to have both the logger call and the page error.
There was a problem hiding this comment.
Perhaps we should allow the user to see the callstack in the error dialog. It can appear as a collapsed area with a toggle.
@ajayalfred in case he has any thoughts here.
There was a problem hiding this comment.
Agree. This will be useful. What do you mean by error dialog? Can you add a screenshot?
There was a problem hiding this comment.
When we show an error dialog like the one below, we can add a collapsed section with the stack trace, and maybe give the user a message to direct them to copy it to any reported bugs.
There was a problem hiding this comment.
Ok sounds good. Do you need a spec for this?
Side note, we should have Jordan do a scrub of text in all dialogs so the messaging is consistent.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rileyjbauer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The only functional change is splitting out the try/catch block in the load function to make the errors more granular and user friendly. This also means the two fetch API calls are done in sequence instead of in parallel, but this isn't a big deal here since it's exactly two calls per page load.
Fixes #379.
This change is