Skip to content

Conversation

@fabianfett
Copy link
Member

Motivation

If we can't establish a connection to a remote for the user, we will retry to create the connection. Eventually the requests connection timeout will fire, and we will fail the request. Right now we fail the request always with the HTTPClientError.getConnectionFromPoolTimeout error. While this is very descriptive, it does not tell the user if there are to many waiting requests or if there was a connection problem.

Changes

  • If errors occur during connection creation, we will save the error in the state machine
  • If a request times out, after we have seen a connection creation error, we will fail the request with the connection creation error.

Result

If user have a typo in their url, they will get better errors. (dns failure)

@fabianfettfabianfett changed the title Preserve connection errors for user[ConnectionPool] Preserve connection errorsSep 10, 2021
@fabianfettfabianfett added this to the HTTP/2 support milestone Sep 10, 2021
@fabianfettfabianfett added semver/none No version bump required. 🔨 semver/patch No public API change. and removed semver/none No version bump required. labels Sep 10, 2021
Copy link
Collaborator

@LukasaLukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM.

privatevarconnections:HTTP1Connections
privatevarfailedConsecutiveConnectionAttempts:Int=0
/// the error from the last connection creation
privatevarlastConnectFailure:Error?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible future enhancement: store a vector of errors and throw a single, composite error that encompasses them all.

@fabianfettfabianfettforce-pushed the ff-better-error-messages branch from 2408ef4 to 701c1d4CompareSeptember 13, 2021 16:14
@fabianfettfabianfett merged commit 0c36de2 into swift-server:mainSep 13, 2021
@fabianfettfabianfett deleted the ff-better-error-messages branch September 13, 2021 16:22
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patchNo public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@fabianfett@Lukasa