fix(net-websockets): ClientWebSocket gains .NET's outer gate (#2357)

Every operation raised WebSocketException(InvalidState) when the instance was
disposed or had never connected. .NET raises ObjectDisposedException and
InvalidOperationException there.

THE TICKET'S FRAMING WAS TOO SIMPLE AND THE REFERENCE CORRECTED IT. It said .NET
'never' raises WebSocketException at these doors. .NET has TWO layers: the outer
gate ConnectedWebSocket (ClientWebSocket.cs:163-177) raises ObjectDisposed /
InvalidOperation, while the inner per-operation check
(WebSocketStateHelper.cs:21-41) raises exactly WebSocketException(InvalidState).
This port's WebSocket::ThrowOnInvalidState IS that inner layer -- a faithful
counterpart of .NET's own protected static -- and was already correct. Rewriting
it would have replaced a correct exception with a wrong one. The outer layer did
not exist here at all.

NO NEW DATA MEMBER WAS NEEDED, so sizeof stays 424 and this is not an SA-3
change: Abort() calls Dispose() in .NET, so Aborted is Disposed there, and
socket_ is assigned only on a successful connect and cleared only by Dispose().
CloseAsync deliberately does not clear it, matching .NET.

#2096's pin is inverted as that ticket anticipated -- it raised WebSocketException
on both sides of the Dispose race precisely because matching .NET on one side
only would be worse, and named #2357 as the ticket that would move the family.
CloseAsync after Dispose used to succeed as a no-op and now faults, which is
.NET's behaviour.

Four mutations, all caught. Gate 17,282 run, 0 failed. Downstream: zero
WebSockets sites in either consumer.

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,76 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — `ClientWebSocket`'s outer gate raises .NET's exceptions (ticket #2357)
+
+*2026-08-18.* Every `ClientWebSocket` operation raised `WebSocketException(InvalidState)` when
+the instance was disposed or had never connected. .NET raises `ObjectDisposedException` and
+`InvalidOperationException` there.
+
+Landed under SA-5 on the user's decision of the same date. **The exception type changes** at five
+doors. No signature, layout, vtable or `noexcept` change — and, notably, **no new data member**:
+`sizeof(ClientWebSocket)` is unchanged at 424, so this is not an SA-3 change.
+
+---
+
+## 1. The ticket's framing was too simple, and the reference corrected it
+
+The ticket reported that .NET raises `ObjectDisposedException` or `InvalidOperationException` and
+that *"neither type is ever `WebSocketException` there"*. .NET actually has **two layers**, and
+only the outer one avoids `WebSocketException`:
+
+| Layer | Where | Raises |
+|---|---|---|
+| **outer** | `ClientWebSocket.ConnectedWebSocket` (`ClientWebSocket.cs:163-177`) | `ObjectDisposedException` if disposed; `InvalidOperationException("The WebSocket is not connected.")` if never connected or still connecting |
+| **inner** | `WebSocketStateHelper.ThrowIfInvalidState` (`WebSocketStateHelper.cs:21-41`), per operation | **`WebSocketException(InvalidState)`** when the current state forbids the operation |
+
+This port had only the inner layer — `WebSocket::ThrowOnInvalidState`, which is a faithful
+counterpart of .NET's own `protected static WebSocket.ThrowOnInvalidState` and **was already
+correct**. Rewriting it would have replaced a correct exception with a wrong one. The outer layer
+did not exist here at all, and is what this ticket adds.
+
+## 2. What changed
+
+| Situation | Was | Is |
+|---|---|---|
+| `SendAsync` / `ReceiveAsync` / `CloseAsync` / `CloseOutputAsync` after `Dispose()` or `Abort()` | `WebSocketException(InvalidState)` | **`ObjectDisposedException`** |
+| the same before `ConnectAsync` completes | `WebSocketException(InvalidState)` | **`InvalidOperationException`**, *"The WebSocket is not connected."* |
+| `CloseAsync` after `Dispose()` | **succeeded as a no-op** | `ObjectDisposedException` |
+| a state that forbids the operation on a **live** socket | `WebSocketException(InvalidState)` | **unchanged** |
+| the `Dispose()` race in `socketForIo()` | `WebSocketException(InvalidState)` | `ObjectDisposedException` — still matching the non-racy path, which is what #2096 required |
+
+## 3. No new data member was needed
+
+.NET's `InternalState` maps exactly onto state this class already holds, because `Abort()` calls
+`Dispose()` (`ClientWebSocket.cs:179-193`) — so *Aborted is Disposed* there — and because
+`socket_` is assigned only on a successful connect and cleared only by `Dispose()`:
+
+| `InternalState` | condition here |
+|---|---|
+| `Created` | `!connectStarted_` |
+| `Connecting` | `connectStarted_ && !socket_ && state_ == Connecting` |
+| `Disposed` | `connectStarted_ && !socket_ && state_ != Connecting` |
+| `Connected` | `socket_ != nullptr` |
+
+`CloseAsync` deliberately does **not** clear `socket_`, matching .NET, where a closed socket is
+still `InternalState.Connected` and the *inner* layer reports the state.
+
+## 4. To migrate
+
+```cpp
+try { ws.SendAsync(...).Wait(); }
+catch (const WebSocketException&) { /* was reached for a disposed socket */ }
+
+// now:
+catch (const System::ObjectDisposedException&) { /* disposed */ }
+catch (const System::InvalidOperationException&) { /* never connected */ }
+catch (const WebSocketException&) { /* live socket, wrong state -- unchanged */ }
+```
+
+If you called `CloseAsync` on a disposed instance and relied on it succeeding, guard it or drop
+it: `Dispose()` has already closed the connection.
+
+## 5. Downstream, measured
+
+Neither `cna` nor `mobile-eggbert` references `System::Net::WebSockets` — **zero sites in both**.
+Neither repository was modified.

docs/Migration-WebSocketExceptionTypes.md

@@ -118,6 +118,11 @@ namespace System::Net::WebSockets {
         void throwIfKeepAliveFaulted() const;
         /** @return A strong reference to the heartbeat state, or null when there is none. */
         [[nodiscard]] std::shared_ptr<KeepAlive> keepAliveState() const;
+        /// #2357: .NET's outer `ConnectedWebSocket` gate -- ObjectDisposedException when this
+        /// instance is disposed, InvalidOperationException when it was never connected or is
+        /// still connecting. Adds no data member; see the definition for the state mapping.
+        void throwIfNotConnected() const;
+
         void sendFrame(SharpRuntime::bytecs opcode, const SharpRuntime::bytecs* data, size_t len, bool fin);
         struct RawFrame {
             SharpRuntime::bytecs opcode = 0;

modules/net-websockets/include/System/Net/WebSockets/ClientWebSocket.hpp

@@ -2,6 +2,7 @@
 // Copyright (c) Robert Vokac and contributors
 // Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
 #include "System/Net/WebSockets/ClientWebSocket.hpp"
+#include "System/ObjectDisposedException.hpp"
 #include <condition_variable>
 #include <memory>
 #include <mutex>
@@ -294,15 +295,60 @@ void ClientWebSocket::storeState(WebSocketState next) {
     state_ = next;
 }
 
+// #2357: .NET's OUTER gate, `ClientWebSocket.ConnectedWebSocket` (`ClientWebSocket.cs:163-177`).
+//
+// THE TICKET'S FRAMING WAS TOO SIMPLE, AND THE REFERENCE CORRECTS IT. It reported that every door
+// here raises WebSocketException(InvalidState) where .NET "never" does. .NET actually has TWO
+// layers, and only the outer one avoids WebSocketException:
+//
+//   * OUTER -- `ConnectedWebSocket`: ObjectDisposedException if the instance is disposed,
+//     InvalidOperationException("The WebSocket is not connected.") if it was never connected or
+//     is still connecting. This layer did not exist here at all.
+//   * INNER -- `WebSocketStateHelper.ThrowIfInvalidState` (`WebSocketStateHelper.cs:21-41`), run
+//     per operation by ManagedWebSocket: WebSocketException(InvalidState) when the CURRENT state
+//     forbids the operation. That is exactly what this port's WebSocket::ThrowOnInvalidState
+//     already did, and it is KEPT. Rewriting it would have replaced a correct exception with a
+//     wrong one.
+//
+// NO NEW DATA MEMBER IS NEEDED, so sizeof(ClientWebSocket) is unchanged and this is not an SA-3
+// change. .NET's InternalState maps exactly onto state this class already holds, because
+// `Abort()` calls `Dispose()` (`ClientWebSocket.cs:179-193`) so Aborted IS Disposed there, and
+// because `socket_` is assigned only on a successful connect and cleared only by Dispose():
+//
+//     Created    !connectStarted_
+//     Connecting  connectStarted_ && !socket_ && state_ == Connecting
+//     Disposed    connectStarted_ && !socket_ && state_ != Connecting
+//     Connected   socket_ != nullptr        (CloseAsync does NOT clear it, matching .NET, where
+//                                            a closed socket is still InternalState.Connected and
+//                                            the INNER layer reports the state)
+void ClientWebSocket::throwIfNotConnected() const {
+    bool disposed = false;
+    {
+        std::lock_guard<std::mutex> lock(stateMutex_);
+        if (socket_) return;                       // Connected -- the inner check decides
+        disposed = connectStarted_ && state_ != WebSocketState::Connecting;
+    }
+    if (disposed) {
+        throw System::ObjectDisposedException("System.Net.WebSockets.ClientWebSocket");
+    }
+    throw System::InvalidOperationException("The WebSocket is not connected.");
+}
+
 std::shared_ptr<System::Net::Sockets::Socket> ClientWebSocket::socketForIo() const {
     std::shared_ptr<System::Net::Sockets::Socket> socket;
     {
         std::lock_guard<std::mutex> lock(stateMutex_);
         socket = socket_;
     }
     if (!socket) {
-        throw WebSocketException(WebSocketError::InvalidState,
-                                  "The WebSocket is in an invalid state for this operation.");
+        // #2357: this is reached when Dispose()/Abort() took the socket away underneath an
+        // operation that had already passed the outer gate -- so it is the DISPOSED case, and
+        // throwIfNotConnected() names it as such. #2096 deliberately raised
+        // WebSocketException(InvalidState) here to match the non-racy path; now that the non-racy
+        // path raises ObjectDisposedException, matching it means raising that. The two sides of
+        // the race still agree, which is what #2096 required.
+        throwIfNotConnected();
+        throw System::ObjectDisposedException("System.Net.WebSockets.ClientWebSocket");
     }
     return socket;
 }
@@ -1007,6 +1053,7 @@ ClientWebSocket::SendAsync(const std::vector<bytecs>& buffer, intcs offset, intc
         AsyncOperationScope release{ops};
         CancellationScope cancellation{this, cancellationToken};
         try {
+            throwIfNotConnected();   // #2357: the OUTER gate, before the per-operation state check
             WebSocket::ThrowOnInvalidState(loadState(), {WebSocketState::Open, WebSocketState::CloseReceived});
             bytecs opcode;
             if (sendContinuation_) {
@@ -1037,6 +1084,7 @@ ClientWebSocket::ReceiveAsync(std::vector<bytecs>& buffer, intcs offset, intcs c
         AsyncOperationScope release{ops};
         CancellationScope cancellation{this, cancellationToken};
         try {
+        throwIfNotConnected();   // #2357: the OUTER gate, before the per-operation state check
         WebSocket::ThrowOnInvalidState(loadState(), {WebSocketState::Open, WebSocketState::CloseSent});
 
         if (recvLeftoverPos_ < recvLeftover_.size()) {
@@ -1166,6 +1214,7 @@ ClientWebSocket::CloseOutputAsync(WebSocketCloseStatus closeStatus, const std::o
         AsyncOperationScope release{ops};
         CancellationScope cancellation{this, cancellationToken};
         try {
+            throwIfNotConnected();   // #2357: the OUTER gate, before the per-operation state check
             WebSocket::ThrowOnInvalidState(loadState(), {WebSocketState::Open, WebSocketState::CloseReceived});
             sendCloseFrame(closeStatus, statusDescription);
             std::lock_guard<std::mutex> lock(stateMutex_);
@@ -1192,6 +1241,11 @@ ClientWebSocket::CloseAsync(WebSocketCloseStatus closeStatus, const std::optiona
         AsyncOperationScope release{ops};
         CancellationScope cancellation{this, cancellationToken};
         try {
+        // #2357: CloseAsync goes through .NET's ConnectedWebSocket gate too, so a DISPOSED
+        // instance faults here rather than treating the call as a no-op. It has no
+        // per-operation ThrowOnInvalidState of its own -- a close on an already-closed but
+        // still-live socket really is a no-op, and stays one.
+        throwIfNotConnected();
         if (loadState() == WebSocketState::Open) {
             sendCloseFrame(closeStatus, statusDescription);
             storeState(WebSocketState::CloseSent);

modules/net-websockets/src/System/Net/WebSockets/ClientWebSocket.cpp

@@ -39,6 +39,9 @@
 #include "System/Net/IPEndPoint.hpp"
 #include "System/Net/Sockets/Socket.hpp"
 #include "System/Net/WebSockets/ClientWebSocket.hpp"
+#include "System/InvalidOperationException.hpp"
+#include "System/ObjectDisposedException.hpp"
+#include "System/Net/WebSockets/WebSocket.hpp"
 #include "System/Net/WebSockets/WebSocketException.hpp"
 #include "System/Net/WebSockets/WebSocketState.hpp"
 #include "System/Uri.hpp"
@@ -105,6 +108,15 @@ class SilentServer {
     SharpRuntime::intcs     port_ = 0;
 };
 
+/// Reaches `WebSocket::ThrowOnInvalidState`, which is `protected static` here exactly as it is in
+/// .NET. Deriving is the language's own way in; widening the production declaration for a test's
+/// convenience is not.
+struct InnerStateCheck : System::Net::WebSockets::WebSocket {
+    static void Check(WebSocketState state, std::initializer_list<WebSocketState> valid) {
+        System::Net::WebSockets::WebSocket::ThrowOnInvalidState(state, valid);
+    }
+};
+
 std::string wsUriFor(SharpRuntime::intcs port) {
     return "ws://127.0.0.1:" + std::to_string(static_cast<int>(port)) + "/";
 }
@@ -194,12 +206,15 @@ TEST(ClientWebSocketConcurrencyTests, Fix2096_TheStatePropertyIsSafeToReadWhileA
     EXPECT_EQ(ws.getStateProperty(), WebSocketState::Closed);
 }
 
-TEST(ClientWebSocketConcurrencyTests, Fix2096_AnOperationAfterDisposeThrowsRatherThanDereferencingNull) {
-    // The non-racy half of the same door: once Dispose() has taken the socket away, every
-    // operation must raise, and it must raise the SAME exception whichever check catches it
-    // first. socketForIo() deliberately reuses WebSocketException(InvalidState) for that reason
-    // -- see its doc-comment, and ticket #2357 for whether the whole family should be
-    // ObjectDisposedException the way .NET's is.
+TEST(ClientWebSocketConcurrencyTests, Fix2357_AnOperationAfterDisposeRaisesObjectDisposedException) {
+    // REWRITTEN BY #2357, WHICH INVERTED THIS PIN. #2096 deliberately raised
+    // WebSocketException(InvalidState) on both sides of the Dispose race, because matching .NET
+    // on one side and not the other would be worse than either -- and it recorded #2357 as the
+    // ticket that would move the whole family at once. It has.
+    //
+    // .NET's outer gate (`ClientWebSocket.cs:163-177`) raises ObjectDisposedException for a
+    // disposed instance and InvalidOperationException for one that was never connected. Neither
+    // is WebSocketException. The two sides of the race still agree, which is what #2096 required.
     SilentServer server;
     ClientWebSocket ws;
     auto task = ws.ConnectAsync(System::Uri(wsUriFor(server.port())));
@@ -209,13 +224,76 @@ TEST(ClientWebSocketConcurrencyTests, Fix2096_AnOperationAfterDisposeThrowsRathe
 
     std::vector<SharpRuntime::bytecs> buffer(16, 0);
     auto send = ws.SendAsync(buffer, 0, 4, System::Net::WebSockets::WebSocketMessageType::Binary, true);
-    EXPECT_THROW(send.Wait(), WebSocketException);
+    EXPECT_THROW(send.Wait(), System::ObjectDisposedException);
 
     auto receive = ws.ReceiveAsync(buffer, 0, 4);
-    EXPECT_THROW((void)receive.getResultProperty(), WebSocketException);
+    EXPECT_THROW((void)receive.getResultProperty(), System::ObjectDisposedException);
 
+    // CloseAsync used to be asserted as a no-op here. It is not one after DISPOSE: .NET routes it
+    // through the same gate, so a disposed instance faults. A close on an already-closed but
+    // still-live socket is still a no-op -- that is a different case, and this row does not
+    // weaken it.
     auto close = ws.CloseAsync(System::Net::WebSockets::WebSocketCloseStatus::NormalClosure, std::nullopt);
-    EXPECT_NO_THROW(close.Wait()) << "a close on an already-closed socket is a no-op, not a fault";
+    EXPECT_THROW(close.Wait(), System::ObjectDisposedException);
+
+    auto closeOutput = ws.CloseOutputAsync(System::Net::WebSockets::WebSocketCloseStatus::NormalClosure,
+                                            std::nullopt);
+    EXPECT_THROW(closeOutput.Wait(), System::ObjectDisposedException);
+}
+
+TEST(ClientWebSocketConcurrencyTests, Fix2357_AnOperationBeforeConnectRaisesInvalidOperationException) {
+    // The gate's other half, which had no counterpart in this port at all: an instance that was
+    // NEVER connected is not disposed, and .NET says so with a different exception and a
+    // different sentence -- "The WebSocket is not connected."
+    ClientWebSocket ws;
+    std::vector<SharpRuntime::bytecs> buffer(16, 0);
+
+    auto send = ws.SendAsync(buffer, 0, 4, System::Net::WebSockets::WebSocketMessageType::Binary, true);
+    EXPECT_THROW(send.Wait(), System::InvalidOperationException);
+
+    auto receive = ws.ReceiveAsync(buffer, 0, 4);
+    try {
+        (void)receive.getResultProperty();
+        ADD_FAILURE() << "expected InvalidOperationException";
+    } catch (const System::ObjectDisposedException&) {
+        ADD_FAILURE() << "a never-connected instance is not a DISPOSED one";
+    } catch (const System::InvalidOperationException& e) {
+        EXPECT_STREQ(e.what(), "The WebSocket is not connected.");
+    }
+
+    auto close = ws.CloseAsync(System::Net::WebSockets::WebSocketCloseStatus::NormalClosure, std::nullopt);
+    EXPECT_THROW(close.Wait(), System::InvalidOperationException);
+}
+
+TEST(ClientWebSocketConcurrencyTests, Fix2357_TheInnerPerOperationStateCheckKeepsWebSocketException) {
+    // THE TICKET'S FRAMING WAS TOO SIMPLE AND THE REFERENCE CORRECTED IT. It reported that .NET
+    // "never" raises WebSocketException at these doors. .NET has TWO layers, and the INNER one --
+    // WebSocketStateHelper.ThrowIfInvalidState (`WebSocketStateHelper.cs:21-41`), run per
+    // operation by ManagedWebSocket -- raises exactly WebSocketException(InvalidState). This
+    // port's WebSocket::ThrowOnInvalidState IS that layer and was already right; rewriting it
+    // would have replaced a correct exception with a wrong one.
+    //
+    // No live socket is used, deliberately. Parking a real connection in CloseSent needs a
+    // cooperating server, and the claim under test is about the LAYER, not about any transport.
+    // The helper is `protected static`, exactly as .NET declares it, so this reaches it through a
+    // local subclass rather than by widening production surface for a test's convenience.
+    EXPECT_THROW(InnerStateCheck::Check(WebSocketState::CloseSent, {WebSocketState::Open}),
+                 WebSocketException);
+    EXPECT_THROW(InnerStateCheck::Check(WebSocketState::None, {WebSocketState::Open}),
+                 WebSocketException);
+    EXPECT_NO_THROW(
+        InnerStateCheck::Check(WebSocketState::Open, {WebSocketState::Open, WebSocketState::CloseReceived}));
+
+    // ...and the inner layer is NOT an ObjectDisposedException door. That is the outer gate's
+    // job, and conflating the two is exactly what this ticket had to avoid.
+    try {
+        InnerStateCheck::Check(WebSocketState::Closed, {WebSocketState::Open});
+        ADD_FAILURE() << "expected a WebSocketException";
+    } catch (const System::ObjectDisposedException&) {
+        ADD_FAILURE() << "the inner state check must not raise ObjectDisposedException";
+    } catch (const WebSocketException&) {
+        SUCCEED();
+    }
 }
 
 TEST(ClientWebSocketConcurrencyTests, Fix2096_AllFiveAsyncMembersJoinTheLivenessBoundary) {