-
Notifications
You must be signed in to change notification settings - Fork 57
Blueprints: Improve error handling for deeplink failures #2461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
0a1bc13 to
9c76a87
Compare
epeicher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ivan-ottinger for improving this! I have tested it, and I can see the new error messages.
Although I'm not sure I like the double Failed to load Blueprint and then The Blueprint could not be loaded. and not displaying the returned error message. I would be interested in others' views on those
| Network Error | Invalid Blueprint error |
|---|---|
![]() |
![]() |
| detail: expect.any( String ), | ||
| buttons: expect.any( Array ), | ||
| message: 'Failed to load Blueprint', | ||
| detail: 'The Blueprint could not be loaded. Please check the link and try again.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Roberto's assessment regarding the wording being repetitive. Maybe The Blueprint could not be loaded could be skipped altogether.
I agree with this assessment, although I think we don't necessarily need the exact error message in this window as as it will be in the logs as well. We should definitely adjust the repetitive wording though. |


Related issues
Proposed Changes
Testing Instructions
npm install && npm start.Pre-merge Checklist