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

Error handler improvements #189

Open
goldbergyoni opened this issue Jul 25, 2022 · 2 comments
Open

Error handler improvements #189

goldbergyoni opened this issue Jul 25, 2022 · 2 comments
Assignees
Labels
needed-now Things we need to finalize a beta

Comments

@goldbergyoni
Copy link
Contributor

goldbergyoni commented Jul 25, 2022

Our handler looks awesome, with that we might have another iteration to perfect it?

What can be improved:

  • The tests should focus on the outcome down to the field level
  • See attached coverage report, some important paths are uncovered
  • We don't test the existence/handling of the last AppError param: cause
  • I would change AppError.isTrusted to AppError.isFatal
  • We lose additional error properties of custom errors: Consider AxiosError.status or MongooseError.someProperty, our handler won't copy these important unknown properties

A. For the case of Error object -> The normalizer can use the origin error object (which already has the stack) and then just thoughtfully copy missing properties:

enrichedError = errorToHandle as AppError
enrichedError.name = errorToHandle.name?? 'General error'
// more properties

B. For the case of non-error object -> We may create AppError, then blend in all the properties from the non-error

const enrichedError = new AppError('General error', ...)
object.assign(enrichedError, errorToHandle)

Or is there a better way? In any case, let's simulate those things in our testing (e.g., non-error with some custom properties)

image

@RonDaha
Copy link
Collaborator

RonDaha commented Jul 26, 2022

@goldbergyoni Thank you, will get into that soon :)

@goldbergyoni goldbergyoni added the needed-now Things we need to finalize a beta label Jul 27, 2022
@marcosmol204
Copy link
Contributor

@RonDaha This feature is still open?
@goldbergyoni What can be my next contribution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needed-now Things we need to finalize a beta
Projects
None yet
Development

No branches or pull requests

3 participants