Closed alecsammon closed 6 months ago
An alternative would be these two PRs:
These would allow us to use errors.Is
for these types.
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 ❤️
There are different types of error that can occur when decoding a request.
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.
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
errors.Is
and determine the correct action to takeHappy 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.