Skip to content
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

feat(client): add details to custom error #1977

Merged
merged 10 commits into from
May 3, 2024

Conversation

daniellacosse
Copy link
Contributor

@daniellacosse daniellacosse commented Apr 17, 2024

This will allow providers to send:

{
  "error": {
    "message": "There was an error.",
    "details": "<Detailed information about the error, displayed in an alert box.>"
  }
}
Screenshot 2024-05-03 at 4 10 36 PM

@daniellacosse daniellacosse added needs test Pull requests that require tests and removed size/XS needs test Pull requests that require tests labels Apr 17, 2024
@daniellacosse daniellacosse reopened this Apr 17, 2024
@daniellacosse daniellacosse marked this pull request as ready for review April 17, 2024 19:18
@daniellacosse daniellacosse requested a review from a team as a code owner April 17, 2024 19:18
Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was quick!

@daniellacosse daniellacosse changed the title feat(client): add cause to custom error feat(client): add details to custom error Apr 25, 2024
client/src/www/app/app.ts Outdated Show resolved Hide resolved
client/src/www/app/app.ts Outdated Show resolved Hide resolved
client/src/www/model/errors.ts Outdated Show resolved Hide resolved
client/src/www/model/errors.ts Outdated Show resolved Hide resolved
client/src/www/model/errors.ts Outdated Show resolved Hide resolved
client/src/www/model/errors.ts Outdated Show resolved Hide resolved
client/src/www/model/errors.ts Outdated Show resolved Hide resolved
super(message);

this.details = details;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can directly declare details in the constructor (like constructor(message: string, private readonly details?: string), so we don't need this assignment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I find that particular shortcut confusing, but I can use it

Copy link
Contributor

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks! That's not a big issue though.

Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!
We should post on Reddit and update the doc once it’s released

@daniellacosse daniellacosse merged commit 3e2aedf into master May 3, 2024
23 checks passed
@daniellacosse daniellacosse deleted the daniellacosse/custom_error_details branch May 3, 2024 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants