Server error handling #31
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves error handling for the Verifactu SOAP API by prioritizing response payload inspection over HTTP status codes. Since Verifactu can return server errors using HTTP status 299 with fault details in the payload, the error detection logic now checks for Body.Fault first and distinguishes between server errors (env:Server code) and validation errors.
Key changes:
- Reordered error checking to inspect response payload before HTTP status code
- Added special handling for server errors with code
env:Serverto returnErrConnectioninstead ofErrValidation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
samlown
left a comment
samlown
left a comment
There was a problem hiding this comment.
Feels a bit confusing for future selves :-)
| } | ||
| return nil, ErrValidation.WithMessage(out.Body.Fault.Message).WithCode(out.Body.Fault.Code) | ||
| } | ||
| if res.StatusCode() != http.StatusOK { |
There was a problem hiding this comment.
This feels a bit hard to read. The code here would suggest that its possible to have a 200 OK response alongside a fault, is that the case? (I understand its not) I'd also suggest adding a few comments as to why the decisions have been made like this so that its easier to understand in the future.
There was a problem hiding this comment.
Yes, it is exactly the issue we had the other day. Not a 200, but a 299. That is why I reversed the errors. I will add comments to make it clearer.
Verifactu returns server errors using HTTP status 299, with the actual server error included in the response payload.
This PR updates the error-handling flow so that we first inspect the response payload for Body.Fault before checking the HTTP status code. If the payload indicates a server error with the code env:Server, we now return a ErrConnection