Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwrap DecoderExceptions on connections (OriginConnectExceptions) into RequestAttempts #1727

Merged
merged 2 commits into from Jan 19, 2024

Conversation

gavinbunney
Copy link
Contributor

@gavinbunney gavinbunney commented Jan 19, 2024

During origin connection failures, netty throws a DecoderException. Currently it is passed through to RequestAttempt and falls into the basic OriginConnectException handling, that is, only the base class name is emitted.

This change unwraps the DecoderException before creating the OriginConnectException so the underlying cause is better propagated. This change also then further unwraps theOriginConnectException in the RequestAttempt class during ssl handshake failures so we know the actual cause of the ssl handshake failure.

Before:

{
  "error": "ORIGIN_CONNECT_ERROR",
  "exceptionType": "DecoderException",
  "status": -1,
  "duration": 42266,
  "attempt": 1,
  "region": "us-east-1",
  "availabilityZone": "us-east-1c",
  "asg": "myapp-v103",
  "instanceId": "i-123abc",
  "vip": "myapp-",
  "ipAddress": "10.0.0.1",
  "port": 443,
  "readTimeout": 600000,
  "connectTimeout": 1500
}

After:

{
  "error": "ORIGIN_CONNECT_ERROR",
  "cause": "Certificate application name (myapp) does not match expected name: mysecondapp",
  "exceptionType": "SSLHandshakeException",
  "status": -1,
  "duration": 42266,
  "attempt": 1,
  "region": "us-east-1",
  "availabilityZone": "us-east-1c",
  "asg": "myapp-v103",
  "instanceId": "i-123abc",
  "vip": "myapp-",
  "ipAddress": "10.0.0.1",
  "port": 443,
  "readTimeout": 600000,
  "connectTimeout": 1500
}

Copy link
Contributor

@jguerra jguerra left a comment

Choose a reason for hiding this comment

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

LGTM

@gavinbunney gavinbunney merged commit 715ee4e into master Jan 19, 2024
5 checks passed
@gavinbunney gavinbunney deleted the gavin/unwrap-oce branch January 19, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants