Currently, we don’t follow any good practice regarding error handling in Cristal. This thread is here to brainstorm some ideas about how to approach this problem.
I would personally vouch for the following approach:
Only ever use throw for non-recoverable errors (e.g. invalid non-type-checkable parameters were provided to the function, such as a negative or floating-point number where a positive integer was expected)
Never ever use throw outside of this, as we can’t see in a function’s signature whether it may throw or not, in which context, and what type of errors it may throw
Fallible functions normally returning a value of type T should return a T | Error (or a specified Error sub-class), to indicate they may fail and allow explicit error handling, or null when the function doesn’t actually fail per se but instead must return nothing
When precise error comprehension is required, return an object akin to { type: ‘success’, value: T } | { type: ‘error’, content: /* Your complex error type here */ }
Document in the function’s TSDoc why it may fail
Never ever throw inside async functions, as await-ing will automatically throw an error and it’s error-prone. Return a proper error instead.
Never ever throw inside a non-async function returning a Promise as intuitively we tend to think of Promise-based functions to throw inside the Promise itself, not outside of it.
For functions that may throw and we have no control over, use the tryFallible and tryFallibleOrError helpers that can be found in @xwiki/cristal-fn-utils
For promises that may reject and we have no control over, use the tryFalliblePromiseOrError helper from the same package
This is quite an extensive list. The most important point is never throwing for recoverable errors, and careful considering what a non-recoverable may be. The other rules are here to provide end-to-end type safety, usability and reduce the risk of mishandling fallible operations.
What do you think about these? Do you have any other suggestion?
I’m not sure if I understand that one. Can you give an example?
In addition, wdyt of defining a pattern for the Error type? The goal is to always have access to the cause of the error (possibly nested when re-returning an error because of an internal error).
It could also be interesting to consider the display of error in natural language (i.e., translatable). I’m mentioning this because we have several usability bugs because of too technical errors being presented to the end user.
ok, reading it again, this refers to rule #1 and rephrases the same thing in a negative form. Do I understand it correctly?
I mean a standardized way to define error types. If we let each developer define error types without a common rule, we’ll end up with a wide variety of solutions, which will be very difficult to maintain.
ok, reading it again, this refers to rule #1 and rephrases the same thing in a negative form. Do I understand it correctly?
Right, my point was badly-worded.
I mean a standardized way to define error types. If we let each developer define error types without a common rule, we’ll end up with a wide variety of solutions, which will be very difficult to maintain.
Yes, I think we should define a CristalError class that extends Error. And then specific errors that contain specific content should extend CristalError. That way we notably have backtraces for free.