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

[alecsammon]

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]

An alternative would be these two PRs:

• Expose core.ErrUnsupportedType to clients to use errors.Is() #90
• Add custom ErrUnknownBodyFormat #91

These would allow us to use errors.Is for these types.

[ggicci]

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

Yes. Sorry I removed the declared errors from the later changes. Because I thought nobody cares about the concrete errors. Apparently It proves to be wrong. Let's bring them back, while I don't want to expose them under httpin package. I still think most of the package users don't care about the concrete errors. Let's put them under core package.

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

I would not suggest classifying errors to client/server from httpin side. Declared errors should suffice and users will be free to classify in their own way as long as we have the minimum granularity 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.

I'm glad that you would like to contribute to this. Thank you ❤️