httpin icon indicating copy to clipboard operation
httpin copied to clipboard

Determining type of error - i.e. client or implementation.

Open alecsammon opened this issue 5 months ago • 2 comments

There are different types of error that can occur when decoding a request.

  1. Client errors where invalid data was passed, i.e. a missing required field
  2. Implementation errors - i.e. where a decoder is not registered.

httpin used to have declared errors, this meant that we could switch on the type of error.

So for "internal/implementation errors" we could then handle these and return a 500 and alert engineers so we could implement a fix.

// internalErrors are system level things that the client can not fix, we return internal
// errors for these and do not surface the route cause.
// httpin has a number of errors, some of these are panics that are caused when registering types.
// The following errors are ones that are evaluated at runtime when the handler structs have been misconfigured.
var internalErrors = []error{
	httpin.ErrDecoderNotFound,
	httpin.ErrMissingDecoderName,
	httpin.ErrUnregisteredExecutor,
}

And the remaining errors were then assumed to be client errors, so we would then return an appropriate 400 error code allowing the client to understand their error.

With the latest versions of httpin then it is much harder to determine the cause of these errors - meaning it's hard to determine whether to return a 500 or a 400 to the client - and what we should log.

How would you feel about

  1. Having a couple of base errors - i.e. Client Error and Internal Error, that are wrapped. This would allow us to do a errors.Is and determine the correct action to take
  2. Using declared Errors again so that custom error handling can be specific with how it manages different types of errors.

Happy to try and put together some PRs if you think this would be ok - or maybe you have some other idea about how this could be achieved.

alecsammon avatar Jan 10 '24 19:01 alecsammon