seasonedcc / composable-functions

Types and functions to make composition easy and safe
MIT License
649 stars 13 forks source link

Simplify error type #132

Closed diogob closed 3 months ago

diogob commented 6 months ago

This PR brings a breaking change proposed for the next major version. It drops the optional exception property present in ErrorWithMessage in favour of always using an Error type to represent errors.

The Error already carries a message, hence once the exception becomes mandatory the wrapping object becomes entirely redundant. Moreover, having an optional exception does not make a lot of sense since the only way to return a result with the errors property is by using exceptions.

The code-base gets a bit tidier and checking for exception types becomes simpler. There is also didactic value since every TS developer should already be familiar with Error.

gustavoguichard commented 6 months ago

I need to think through it. My main concern is how easy is it to work with at the UI level, are we gonna have helpers to extract the messages? Also, how does it affect InputErrors, EnvironmentErrors, and other abstractions people may be using.

diogob commented 6 months ago

Using some examples from a real code base:

Expressions relying on messages would be unchanged, such as

errorData.errors.some(({ message }) => message.includes('team_name_key'))

This works since the property message exists both in our existing ErrorWithMessage and in the native Error. We wouldn't need any helper for that, although that might be an interesting idea to tighten the abstraction.

The expression

Array.isArray(error.errors)

Would also be unchanged, or anything using the list interface.

The breaking changes happen on code that deals with the exception property, such as

'data' in error.errors[0].exception

Would have to be rewritten as

'data' in error.errors[0]

and

result.errors.find((e) => e.exception instanceof UnauthorizedError),

Would have to be rewritten as

result.errors.find((e) => e instanceof UnauthorizedError),
diogob commented 6 months ago

InputErrors and EnvironmentErrors will no be affected as far as I can tell, since they deal with SchemaError.

diogob commented 6 months ago

A smoother approach would be to make exception mandatory and create a function to access the exception (and perhaps the message). This would make for a tighter abstraction and easier future changes.

gustavoguichard commented 6 months ago

Ok, I think I like it, I've been dealing with errors[i].exception a lot lately anyway

diogob commented 3 months ago

composable-functions is upon us!