JAVA-6194 Add MongoSocksProxyException for CMAP backpressure labeling by nhachicha · Pull Request #1968 · mongodb/mongo-java-driver

nhachicha · 2026-05-08T17:41:13Z

…ABEL` (mongodb#1926) This commit only adds the labels, and does not fully implement the tickets specified below. The reason there are four JAVA tickets specified is that the is a single specification commit that resolved the four corresponding DRIVERS tickets. All of these JAVA tickets have to be done together. The relevant spec changes: - https://github.com/mongodb/specifications/blame/ba14b6bdc1dc695aa9cc20ccf9378592da1b2329/source/client-backpressure/client-backpressure.md#L52-L80 - it's a subset of [DRIVERS-3239, DRIVERS-3411, DRIVERS-3370, DRIVERS-3412: Client backpressure (mongodb#1907)](mongodb/specifications@1125200) JAVA-5956, JAVA-6117, JAVA-6113, JAVA-6119

…b#1929) The relevant spec changes: - https://github.com/mongodb/specifications/blame/ba14b6bdc1dc695aa9cc20ccf9378592da1b2329/source/retryable-writes/tests/README.md#L265-L418 - See also https://jira.mongodb.org/browse/DRIVERS-3432 for the phrasing fixes for "Test 3 Case 3" JAVA-6055

JAVA-6141

- Deprioritize sharded clusters on any error, all other topologies only on SystemOverloadedError. - Pass ClusterType to updateCandidate so onAttemptFailure can distinguish topology types. - Add retryable reads prose tests 3.1 and 3.2. - Change ServerSelectionSelectionTest to use BaseCluster server selection chain. JAVA-6105 JAVA-6021 JAVA-6074 --------- Co-authored-by: Valentin Kovalenko <[email protected]> Co-authored-by: Ross Lawley <[email protected]>

The relevant spec changes: - https://github.com/mongodb/specifications/blob/8a8a7c56429c80b51ec62268dcafc5e5e3c477ef/source/client-backpressure/tests/README.md JAVA-5956, JAVA-6117, JAVA-6113, JAVA-6119, JAVA-6141

- Add enableOverloadRetargeting boolean option to MongoClientSettings and ConnectionString to allow the driver to route requests to a different replica set member on retries when the previously used server is overloaded - Add prose test 3.3 to verify that overload errors are retried on the same server when retargeting is disabled JAVA-6167 --------- Co-authored-by: Ross Lawley <[email protected]>

…mongodb#1949)

… on retries (mongodb#1899)

Copilot

Pull request overview

This PR introduces a dedicated MongoSocksProxyException to represent SOCKS5 proxy connection/handshake failures (with phase + optional RFC 1928 reply code), and updates the SOCKS implementation/tests to throw and assert this new exception type. This supports more precise classification/handling of SOCKS-related connection failures (per JAVA-6194).

Changes:

Added MongoSocksProxyException (with HandshakePhase and optional proxy reply code).
Updated SocksSocket and SocketStream to throw/propagate MongoSocksProxyException for SOCKS negotiation/auth/connect failures and proxy TCP connect failures.
Updated/added tests to validate the new exception type and its phase/reply-code tagging.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
driver-sync/src/test/functional/com/mongodb/client/Socks5ProseTest.java	Updates prose test assertions to expect `MongoSocksProxyException` during SOCKS auth failures.
driver-core/src/test/unit/com/mongodb/internal/connection/SocksSocketTest.java	Adds unit tests validating handshake phase tagging and CONNECT reply-code tagging.
driver-core/src/main/com/mongodb/MongoSocksProxyException.java	Adds new public exception type carrying SOCKS handshake phase and optional RFC 1928 reply code.
driver-core/src/main/com/mongodb/internal/connection/SocksSocket.java	Replaces some SOCKS protocol failures with `MongoSocksProxyException` and adds helper to build a `ServerAddress`.
driver-core/src/main/com/mongodb/internal/connection/SocketStream.java	Wraps proxy-enabled IO failures as `MongoSocksProxyException` and rethrows SOCKS exceptions directly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…re/socks5_exception

- Delegate single-arg constructor to four-arg variant - Clarify Javadoc that proxyReplyCode may be null - Remove duplicate blank lines between constructors

MongoSocksProxyException extends RuntimeException and bypassed the existing catch (SocketException) block, leaking the underlying proxy TCP socket on every SOCKS5 protocol failure (negotiation/auth/relay).

Defensive: avoids any reverse-DNS risk in error paths if the unresolved invariant on remoteAddress is ever weakened.

- Close inner socket on failure inside initializeSocketOverSocksProxy and initializeSslSocketOverSocksProxy so that failures before this.socket is assigned do not leak the underlying file descriptor. - Hoist translateInterruptedException out of the two orElseThrow branches.

Server thread now blocks on the client closing the connection instead of guessing how long the SOCKS exchange will take. Removes a CI flake under load.

Port 1 (tcpmux) is unassigned on most systems but not guaranteed. Binding then releasing an ephemeral port gives a reliably-closed port.

The previous fix used InputStream.transferTo (Java 9+) and OutputStream.nullOutputStream (Java 11+), which CI on Java 8 surfaced as NoSuchMethodError. The test source set is compiled by the Groovy compiler (because src/test/unit contains both Java and Groovy), which does not honor options.release.set(8) — Java 11 API calls slipped past compile-time validation but failed at link time on Java 8. Replace with a plain read-into-discard-buffer loop. Same semantics (blocks until client closes), Java 8 source compatible.

ApiAliasAndCompanionSpec discovers every public subtype of MongoException in the Java driver and asserts each one has a corresponding Scala alias. MongoSocksProxyException is new in 5.8 and was missing from the wrapper.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

…eler JAVA-6194: SOCKS5 failures during post-TCP phases (negotiation, auth, CONNECT-relay) are configuration/protocol errors and must not carry SystemOverloadedError or RetryableError labels. Failures during the PROXY_TCP_CONNECT phase are plain TCP-level reach failures (proxy host unreachable / overloaded) and continue to receive the labels like any other socket-open failure. Removes the corresponding TODO.

…in SocksSocket.connect

vbabanin · 2026-05-22T05:25:12Z

+        } catch (IOException | RuntimeException e) {
+            try {
+                toClose.close();
+            } catch (IOException closeException) {
+                e.addSuppressed(closeException);
+            }
+            throw e;


Good catch!

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

vbabanin · 2026-05-22T23:59:31Z

+
+            try (SocksSocket socksSocket = new SocksSocket(buildProxySettings("127.0.0.1", port, withCredentials))) {
+                try {
+                    socksSocket.connect(TARGET, 5000);


This constant appears multiple times. Let's move it to CONNECT_TIMEOUT_MS?

vbabanin · 2026-05-23T00:01:53Z

+                0x05, 0x04, 0x00, 0x01, 0, 0, 0, 0, 0, 0   // HOST_UNREACHABLE
+        };
+        MongoSocksProxyException ex = assertProxy(connectWithMiniServer(bytes, false));
+        Assertions.assertNotNull(ex);


Many tests do:

MongoSocksProxyException ex = assertProxy(connectWithMiniServer(...)); Assertions.assertNotNull(ex);

assertProxy uses assertInstanceOf(...), which already fails on null, so the extra assertNotNull is redundant and could be removed.

vbabanin · 2026-05-23T00:15:25Z

+                } catch (MongoSocksProxyException e) {
+                    return e;
+                }


connectWithMiniServer currently catches the exception and returns it, which duplicates what JUnit already provides. We can use the standard JUnit idiom to both assert and capture the exception:

MongoSocksProxyException ex = assertThrows(MongoSocksProxyException.class, () -> socksSocket.connect(TARGET, 5000));

This removes the manual catch/return flow and keeps the intent (“this must throw”) explicit.

vbabanin · 2026-05-23T00:16:49Z

+    }
+
+    private static MongoSocksProxyException assertProxy(final Exception ex) {
+        return assertInstanceOf(MongoSocksProxyException.class, ex,


See https://github.com/mongodb/mongo-java-driver/pull/1968/changes#r3291696461.

vbabanin · 2026-05-23T00:22:05Z

+        // Negotiation succeeds picking USERNAME_PASSWORD; mini-server then half-closes immediately,
+        // so the client reads the 2 negotiation bytes successfully and then sees EOF on the
+        // subsequent auth-result read. readSocksReply throws ConnectException("Malformed reply...")
+        // from inside authenticate(). The wrapper must surface this as MongoSocksProxyException
+        // with no reply code.


This comment describes internal implementation details (readSocksReply, a specific ConnectException message, “the wrapper surfaces this”) rather than the behavior the test is asserting. The test should document the observable contract. If the internals change while the behavior stays the same, the comment becomes misleading and will need unnecessary updates.

I suggest removing it (or rewriting it to describe only the expected externally visible behavior).

vbabanin · 2026-05-23T00:25:02Z

+        // Mini-server half-closes immediately after writing zero bytes of method-selection reply.
+        // Client's readSocksReply sees EOF (in.read() == -1) and throws ConnectException("Malformed
+        // reply..."). That IOException must be wrapped as MongoSocksProxyException with no reply
+        // code (failure happened before any CONNECT reply was parsed).


Same as in https://github.com/mongodb/mongo-java-driver/pull/1968/changes#r3291708937.

readSocksReply, in.read() == -1, and ConnectException("Malformed reply...") are all internal to SocksSocket and might change.

…sSocketFunctionalTest.java Co-authored-by: Viacheslav Babanin <[email protected]>

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

…edException

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

…re/socks5_exception

nhachicha and others added 10 commits March 5, 2026 13:34

JAVA-6035: Add backpressure flag to connection handshake (mongodb#1906)

ac1c120

Add maxAdaptiveRetries API (mongodb#1944)

ffe5242

JAVA-6141

Implement prose backpressure tests (mongodb#1946)

c380b2e

The relevant spec changes: - https://github.com/mongodb/specifications/blob/8a8a7c56429c80b51ec62268dcafc5e5e3c477ef/source/client-backpressure/tests/README.md JAVA-5956, JAVA-6117, JAVA-6113, JAVA-6119, JAVA-6141

Add handshake prose Test 9: backpressure: true in handshake documents (…

bd888c9

…mongodb#1949)

JAVA-5950 Update Transactions Convenient API with exponential backoff…

394f7b1

… on retries (mongodb#1899)

JAVA-6194 Add MongoSocksProxyException for CMAP backpressure labeling

0eb04a0

nhachicha self-assigned this May 8, 2026

nhachicha requested a review from Copilot May 8, 2026 17:41

Copilot started reviewing on behalf of nhachicha May 8, 2026 17:42 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

stIncMale force-pushed the backpressure branch 2 times, most recently from 2d1b2e1 to 11a7b73 Compare May 8, 2026 20:58

nhachicha added 10 commits May 16, 2026 18:38

Merge remote-tracking branch 'origin/backpressure' into nh/backpressu…

f2bcce5

…re/socks5_exception

update submodule

28a074d

Address review nits in MongoSocksProxyException

c2ca4fd

- Delegate single-arg constructor to four-arg variant - Clarify Javadoc that proxyReplyCode may be null - Remove duplicate blank lines between constructors

Close proxy socket on MongoSocksProxyException in SocksSocket.connect

801127f

MongoSocksProxyException extends RuntimeException and bypassed the existing catch (SocketException) block, leaking the underlying proxy TCP socket on every SOCKS5 protocol failure (negotiation/auth/relay).

Use getHostString in SocksSocket exception reporting path

61a1c5e

Defensive: avoids any reverse-DNS risk in error paths if the unresolved invariant on remoteAddress is ever weakened.

Replace Thread.sleep(300) with input drain in SocksSocketTest

db26d92

Server thread now blocks on the client closing the connection instead of guessing how long the SOCKS exchange will take. Removes a CI flake under load.

Use ephemeral closed port instead of port 1 in SocksSocketTest

fd39744

Port 1 (tcpmux) is unassigned on most systems but not guaranteed. Binding then releasing an ephemeral port gives a reliably-closed port.

Add Scala type alias for MongoSocksProxyException

4e3249b

ApiAliasAndCompanionSpec discovers every public subtype of MongoException in the Java driver and asserts each one has a corresponding Scala alias. MongoSocksProxyException is new in 5.8 and was missing from the wrapper.

nhachicha requested a review from Copilot May 18, 2026 15:41

Copilot started reviewing on behalf of nhachicha May 18, 2026 15:41 View session

Copilot AI reviewed May 18, 2026

View reviewed changes

vbabanin reviewed May 21, 2026

View reviewed changes

nhachicha added 2 commits May 21, 2026 15:06

Dropped the HandshakePhase enum

9efad2b

PR feedback

a838766

nhachicha requested a review from vbabanin May 21, 2026 14:58

Wrapping IOException into MongoSocksProxyException to simplify logic …

a1f0cb8

…in SocksSocket.connect

vbabanin reviewed May 22, 2026

View reviewed changes

vbabanin requested a review from Copilot May 22, 2026 23:24

Copilot started reviewing on behalf of vbabanin May 22, 2026 23:24 View session

Copilot AI reviewed May 22, 2026

View reviewed changes

nhachicha and others added 2 commits May 23, 2026 01:03

Potential fix for pull request finding

c188e24

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

Copilot doc nits

91d3d09

vbabanin reviewed May 23, 2026

View reviewed changes

nhachicha and others added 5 commits May 23, 2026 02:11

Update driver-core/src/test/unit/com/mongodb/internal/connection/Sock…

a239532

…sSocketFunctionalTest.java Co-authored-by: Viacheslav Babanin <[email protected]>

Update driver-core/src/test/unit/com/mongodb/internal/connection/Sock…

9e09f96

…sSocketFunctionalTest.java Co-authored-by: Viacheslav Babanin <[email protected]>

Update driver-core/src/test/unit/com/mongodb/internal/connection/Sock…

eb6a716

…sSocketFunctionalTest.java Co-authored-by: Viacheslav Babanin <[email protected]>

Update driver-core/src/test/unit/com/mongodb/internal/connection/Sock…

196ddf1

…sSocketFunctionalTest.java Co-authored-by: Viacheslav Babanin <[email protected]>

Review feedback

882a416

nhachicha requested a review from Copilot May 23, 2026 01:36

Copilot started reviewing on behalf of nhachicha May 23, 2026 01:36 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread driver-core/src/main/com/mongodb/MongoSocksProxyException.java

Comment thread driver-core/src/main/com/mongodb/internal/connection/BackpressureErrorLabeler.java Outdated

Copilot nit

2fe56ff

nhachicha requested a review from Copilot May 23, 2026 01:49

Copilot started reviewing on behalf of nhachicha May 23, 2026 01:49 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread driver-core/src/main/com/mongodb/internal/connection/SocketStream.java

Translate interrupt-flavored proxy-connect failures to MongoInterrupt…

befdfff

…edException

nhachicha requested a review from Copilot May 23, 2026 02:34

Copilot started reviewing on behalf of nhachicha May 23, 2026 02:34 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Merge remote-tracking branch 'origin/backpressure' into nh/backpressu…

7975b3b

…re/socks5_exception

nhachicha requested a review from vbabanin May 23, 2026 02:44

Conversation

nhachicha commented May 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vbabanin May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

vbabanin May 22, 2026 •

edited

Loading