Wrap remaining transport exceptions in Upcheck::ConnectionError#2
Open
Montana wants to merge 1 commit into
Open
Wrap remaining transport exceptions in Upcheck::ConnectionError#2Montana wants to merge 1 commit into
Upcheck::ConnectionError#2Montana wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey all,
Upcheck::HTTPClientrescues timeouts,SocketError, and a fewErrnoclasses, but several other common transport failures leak to callers as raw exceptions, breaking the README's promise that "every failure raises a specific subclass ofUpcheck::Error":OpenSSL::SSL::SSLError— TLS handshake failures (expired/invalid certs, protocol mismatch)Errno::ECONNRESET— connection reset by peer (I reproduced this by pointing the client at a server that closes the socket during the TLS handshake)EOFError— server closes the connection mid-responseNet::HTTPBadResponse— server responds with something that isn't HTTPReproduction against a local socket that accepts and immediately closes connections:
This matters for a status-checking gem in particular: callers rescuing
Upcheck::Erroraround aprovider.statuscall (as the README suggests) will still crash on exactly the kind of network weirdness that tends to accompany an upstream outage.Fix
Add the four exception classes to the existing
ConnectionErrorrescue inHTTPClient#get, andrequire "openssl"explicitly since the rescue now references its constant.All four are connection-level failures, so
ConnectionError(rather than a new subclass) seemed like the right home — happy to split them out if you'd prefer.Testing
Four new examples in
http_client_spec.rbusing WebMock'sto_raise, one per exception class, mirroring the existingSocketErrorexample. Full suite: 95 examples, 0 failures. Also verified against a real local TCP socket (not just WebMock) thatECONNRESETandEOFErrorare now wrapped.