-
Notifications
You must be signed in to change notification settings - Fork 12
Permalink
Choose a base ref
{{ refName }}
default
Choose a head ref
{{ refName }}
default
Comparing changes
Choose two branches to see what’s changed or to start a new pull request.
If you need to, you can also or
learn more about diff comparisons.
Open a pull request
Create a new pull request by comparing changes across two branches. If you need to, you can also .
Learn more about diff comparisons here.
base repository: launchdarkly/java-core
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: launchdarkly-java-sdk-internal-1.11.0
Could not load branches
Nothing to show
Loading
Could not load tags
Nothing to show
{{ refName }}
default
Loading
...
head repository: launchdarkly/java-core
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: launchdarkly-java-sdk-internal-1.11.1
Could not load branches
Nothing to show
Loading
Could not load tags
Nothing to show
{{ refName }}
default
Loading
- 2 commits
- 5 files changed
- 2 contributors
Commits on Aug 28, 2026
-
fix(internal): narrow TLS classification to certificate failures only (…
…#206) ## Summary `FailureClass.hasTlsOrCertificateCause` matched any `SSLException` **or** `GeneralSecurityException` anywhere in the cause chain. Since all SDK traffic is HTTPS, every transport error arrives through the TLS layer — so this swept in transient faults unrelated to certificate validity and classified them `UNEXPECTED`, pushing data sources into extended-regime backoff (5 min – 1 hr). Found during review of #200 by @jsonbailey, who flagged the breadth but could not run a JVM to confirm what JSSE actually throws. Confirmed empirically below. ## The problem, measured Against a real `SSLServerSocket`: | Scenario | JSSE exception | Old classification | |---|---|---| | Peer sends **FIN** mid-handshake | `SSLHandshakeException: Remote host terminated the handshake`<br>← caused by `EOFException: SSL peer shut down incorrectly` | **UNEXPECTED** ❌ | | Peer sends **RST** mid-handshake | `SocketException: Broken pipe` | NORMAL ✓ | | Untrusted certificate chain | `SSLHandshakeException` → `ValidatorException` → `SunCertPathBuilderException` | UNEXPECTED ✓ | The regime therefore depended on whether an intermediary sent FIN or RST — an arbitrary implementation detail. Real triggers for FIN-mid-handshake are all transient: load balancer draining during a rolling restart, a connection-limit polite close, an idle timeout during a slow handshake. **The compounding case is worse than a single stall.** A connection that flaps faster than the 60 s healthy-operation reset window never accumulates enough continuous connectivity to reset, so it ratchets 5 m → 10 m → 20 m → 40 m → 1 hr and stays there. ## The fix Match only genuinely long-lived certificate problems: ```java c instanceof CertificateException // expired, not-yet-valid, hostname mismatch; // also covers ValidatorException || c instanceof CertPathValidatorException // untrusted chain || c instanceof CertPathBuilderException || c instanceof SSLPeerUnverifiedException // hostname mismatch ``` Verified that a genuinely untrusted chain still classifies `UNEXPECTED` — JSSE's `ValidatorException` is a `CertificateException` and `SunCertPathBuilderException` is a `CertPathBuilderException`, so two links of the real chain match. ## Parity with Go This aligns Java with the Go server SDK, whose `classifyTransportFailure` enumerates only certificate errors and treats everything else as normal: ```go tls.CertificateVerificationError x509.UnknownAuthorityError x509.HostnameError x509.CertificateInvalidError // everything else -> FailureClassNormal ``` The previous Java behavior was a divergence from that reference implementation, not a different reading of the spec. ## Tests - **Replaces** `sslHandshakeIsUnexpected`, which asserted the over-broad behavior, with `bareSslHandshakeFailureIsNormal` and `peerClosedMidHandshakeIsNormal` (the latter reproducing the real `SSLHandshakeException` → `EOFException` shape). - **Adds** `certPathValidatorFailureIsUnexpected`, `certPathBuilderFailureIsUnexpected`, `certificateNotYetValidIsUnexpected`, `untrustedChainWrappedInHandshakeExceptionIsUnexpected`, `sslExceptionFromConnectionResetIsNormal`. - Full `lib/shared/internal` suite green; `checkstyleMain` clean. ## Test plan for reviewers - [x] Confirm the four matched types are the right set — in particular that `CertificateException` is the correct catch-all for validator failures, and that nothing in Go's four cases lacks a Java counterpart here. - [x] Consider whether a bare `SSLHandshakeException` with a *cipher/protocol* mismatch cause (e.g. `handshake_failure` alert, "No appropriate protocol") should be `UNEXPECTED`. It is persistent like a cert problem, but it is not a certificate error and is now classified `NORMAL`. I left it as `NORMAL` to avoid re-widening, and because a persistent mismatch keeps retrying at 1–30 s rather than stalling — but it is a judgment call. - [x] Sanity-check that no other caller depends on the old broad behavior. ## Downstream #200 consumes this classifier. It needs an internal release (1.11.1) before it can pick this up, in addition to the `classifyAndLogHttpFailure` rename already noted in review there. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > **Narrows when HTTPS transport failures trigger extended-regime backoff** by changing `FailureClass.hasTlsOrCertificateCause` to walk the exception chain for **certificate validation problems only** (`CertificateException`, `CertPathValidatorException`, `CertPathBuilderException`, `SSLPeerUnverifiedException`), instead of any `SSLException` or `GeneralSecurityException`. > > Because all SDK traffic is TLS, the old rule treated many **transient** handshake faults (e.g. peer FIN mid-handshake, bare `SSLHandshakeException`, connection-reset `SSLException`) as **UNEXPECTED**, which could push data sources into multi-minute backoff. Genuine cert issues (expired/not-yet-valid, untrusted chain wrapped in `SSLHandshakeException`) still classify **UNEXPECTED**. > > Tests are updated to match: removed the expectation that every SSL handshake failure is unexpected, added cases for cert-path errors and normal transient SSL shapes, and kept wrapped-certificate-cause coverage. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fa662f2. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Configuration menu - View commit details
-
Copy full SHA for e76ff16 - Browse repository at this point
Copy the full SHA e76ff16View commit details -
chore(main): release launchdarkly-java-sdk-internal 1.11.1 (#207)
🤖 I have created a release *beep* *boop* --- ## [1.11.1](launchdarkly-java-sdk-internal-1.11.0...launchdarkly-java-sdk-internal-1.11.1) (2026-08-28) ### Bug Fixes * **internal:** narrow TLS classification to certificate failures only ([#206](#206)) ([e76ff16](e76ff16)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Release Please bumps **`lib/shared/internal`** from **1.11.0** to **1.11.1** (manifest, `gradle.properties`, and changelog). > > The release notes the bug fix from [#206](#206): **HTTP transport failure classification** now treats only **certificate / TLS validation** causes (e.g. `CertificateException`, cert path errors, `SSLPeerUnverifiedException`) as **`FailureClass.UNEXPECTED`** with extended backoff. Other SSL or transport errors stay **`NORMAL`**, so they no longer trigger the extended-regime path by mistake. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit da47451. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Configuration menu - View commit details
-
Copy full SHA for 717908b - Browse repository at this point
Copy the full SHA 717908bView commit details
Loading
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff launchdarkly-java-sdk-internal-1.11.0...launchdarkly-java-sdk-internal-1.11.1