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

[Discussion] Handling of errors #165

Open
MatthiasKunnen opened this issue Jan 30, 2021 · 3 comments
Open

[Discussion] Handling of errors #165

MatthiasKunnen opened this issue Jan 30, 2021 · 3 comments

Comments

@MatthiasKunnen
Copy link
Contributor

I want to start a discussion about error handling in the TypedJSON.

In my opinion, TypedJSON is too lax regarding default error behavior.

My proposal:

  • Errors that arise during non-parsing stage, e.g. type not determinable when using @jsonMember, should always throw and not use the error handler.
    There errors are always the fault of a misconfiguration and I believe there to be no reason why you would ever want to keep that in your code.
  • The errorHandler should throw by default. We want to prevent new adopters from bugs silently creeping into their codebase. People that do not mind errors while (de)serializing can overwrite the error handler to go back to the old behavior.

I'd love your opinion @Neos3452.

@Neos3452
Copy link
Collaborator

Neos3452 commented Feb 1, 2021

I totally agree with the first point.

For the second, I have mixed feelings. On a frontend, you want to keep going as far as possible. It is not reasonable to error out on a parsing error, and potentially prevent a user from completing a purchase. On the other hand you want to catch such errors as soon as possible — in dev, or in tests. However, from experience silent errors were a real hinderance when undefined randomly cropped into different layers.

@MatthiasKunnen
Copy link
Contributor Author

Personally, I am a fan of erroring early. Ignoring a problem in an early stage with the hope of things turning out okay almost always causes problems down the line. That being said, I believe we can keep the optimistic behavior available using the custom error handler. People that want the old behavior can use setGlobalConfig({errorHandler: console.error}). I would recommend the default behavior to error, though, for the reasons mentioned point 2 and the silent errors being a hindrance as you said.

Regardless of whether we change the default behavior, both points are a breaking change and will require a new major version.

@MatthiasKunnen
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants