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

POC of error handling #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mynomoto
Copy link

This is only a poc of how I suggest we handle errors. Here we catch and rethrow an ex-info with the original exception as the cause. If the user only uses the top level exception they can replace implementations without having to redo the error handling on the application.

WDYT?

@borkdude
Copy link
Member

borkdude commented Apr 30, 2022

@mynomoto That's an interesting problem and approach, I hadn't thought about uniform error handling yet.

Perhaps functions should be wrapped in a catch-all macro that just returns the original exception in an ex-info? Or perhaps callers should always catch Exception and then deal with the specifics themselves?

@mynomoto
Copy link
Author

mynomoto commented May 1, 2022

@borkdude I think that there are always tradeoffs 😉

How I see this atm:

  • We could do nothing and if users catch anything related to json errors and changes implementations they will need to change what they catch if they use specific errors. They can catch Exception but if are more than one possible error and they need to know the difference it will change and code will need to change.
  • We could catch anything and wrap. It is easy and simple to implement but if users need any information about the error they will need to check the original exception and code will need to change when changing implementations.
  • We could attempt to catch and rethrow specific exceptions but that would be a lot of work. There are many different possible exceptions on clojure.data.json alone, and I'm not sure if there is a mapping between different library exceptions. The data available on the exception is also different with cheshire returning error position but clojure.data.json not.

I don't think that this is very important here because in a certain way invalid json is in the end invalid, the effect is the same as in the app cannot process it. This type of discussion will be more important for an http client because connection errors and timeouts are different in important ways.

Another advantage to having errors defined in this library is making testing easier because we could expose functions that construct the exceptions so users can simulate errors in json processing easily.

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

Successfully merging this pull request may close these issues.

2 participants