From 8ee59ebf5cc5e63723c8c34401f17880998e8688 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Tue, 12 Apr 2022 20:41:45 -0700 Subject: [PATCH 1/4] Add UnplugServer test --- .../WebSocketIntegrationTest.cpp | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp index a540ad82bf0..3a97c0a625d 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp @@ -23,6 +23,35 @@ using Error = IWebSocketResource::Error; TEST_CLASS (WebSocketIntegrationTest) { + TEST_METHOD(UnplugServer) + { + //auto server = make_shared(5555, false); + promise closedPromise; + auto ws = IWebSocketResource::Make(); + string errorMessage{}; + ws->SetOnClose([&closedPromise](CloseCode code, const string& reason) + { + int x = 99; + closedPromise.set_value(); + }); + ws->SetOnMessage([](size_t, const string& message, bool isBinary) + { + int x = 88; + }); + ws->SetOnError([&closedPromise, &errorMessage](IWebSocketResource::Error&& error) + { + errorMessage = std::move(error.Message); + closedPromise.set_value(); + }); + + ws->Connect("ws://localhost:5555"); + + + closedPromise.get_future().wait(); + + Assert::AreEqual({}, errorMessage); + } + void SendReceiveCloseBase(bool isSecure) { auto server = make_shared(5556, isSecure); From 728bbc675f368fd737ffd80c252a826d3cd4cf2f Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 13 Apr 2022 01:39:37 -0500 Subject: [PATCH 2/4] Handle exceptions in args.GetDataReader() --- .../WebSocketIntegrationTest.cpp | 24 +++++++++---------- vnext/Shared/IWebSocketResource.h | 2 +- vnext/Shared/WinRTWebSocketResource.cpp | 22 ++++++++++++++++- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp index 3a97c0a625d..95d5b8f0932 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp @@ -23,25 +23,20 @@ using Error = IWebSocketResource::Error; TEST_CLASS (WebSocketIntegrationTest) { - TEST_METHOD(UnplugServer) + TEST_METHOD(AbruptServerDisconnectFailsWithSpecificMessage) { - //auto server = make_shared(5555, false); promise closedPromise; auto ws = IWebSocketResource::Make(); - string errorMessage{}; - ws->SetOnClose([&closedPromise](CloseCode code, const string& reason) + IWebSocketResource::Error error{{}, IWebSocketResource::ErrorType::None}; + CloseCode closeCode; + ws->SetOnClose([&closedPromise, &closeCode](CloseCode code, const string& reason) { - int x = 99; + closeCode = code; closedPromise.set_value(); }); - ws->SetOnMessage([](size_t, const string& message, bool isBinary) + ws->SetOnError([&closedPromise, &error](IWebSocketResource::Error&& wsError) { - int x = 88; - }); - ws->SetOnError([&closedPromise, &errorMessage](IWebSocketResource::Error&& error) - { - errorMessage = std::move(error.Message); - closedPromise.set_value(); + error = std::move(wsError); }); ws->Connect("ws://localhost:5555"); @@ -49,7 +44,10 @@ TEST_CLASS (WebSocketIntegrationTest) closedPromise.get_future().wait(); - Assert::AreEqual({}, errorMessage); + //NOTE: This message is implementation-specific (WinRTWebSocketResource) + Assert::AreEqual({"[0x80072EFE] Underlying TCP connection suddenly terminated"}, error.Message); + Assert::AreEqual(static_cast(IWebSocketResource::ErrorType::Connection), static_cast(error.Type)); + Assert::AreEqual(static_cast(CloseCode::BadPayload), static_cast(closeCode)); } void SendReceiveCloseBase(bool isSecure) diff --git a/vnext/Shared/IWebSocketResource.h b/vnext/Shared/IWebSocketResource.h index 6c27c72755f..0fff9e20b96 100644 --- a/vnext/Shared/IWebSocketResource.h +++ b/vnext/Shared/IWebSocketResource.h @@ -71,7 +71,7 @@ struct IWebSocketResource { struct Error { std::string Message; - const ErrorType Type; + ErrorType Type; }; #pragma endregion Inner types diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index 7368d505a68..2a4093d77a4 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -296,7 +296,27 @@ void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, c [self = shared_from_this()](IWebSocket const &sender, IMessageWebSocketMessageReceivedEventArgs const &args) { try { string response; - IDataReader reader = args.GetDataReader(); + IDataReader reader = nullptr; + try { + reader = args.GetDataReader(); + } catch (hresult_error const &e) { + string message; + ErrorType errorType; + + // See https://docs.microsoft.com/uwp/api/windows.networking.sockets.messagewebsocketmessagereceivedeventargs.getdatareader?view=winrt-19041#remarks + if (e.code() == WININET_E_CONNECTION_ABORTED) { + message = "[0x80072EFE] Underlying TCP connection suddenly terminated"; + errorType = ErrorType::Connection; + } else { + message = Utilities::HResultToString(e); + errorType = ErrorType::Receive; + } + + self->m_errorHandler({message, errorType}); + + return self->Close(CloseCode::BadPayload, message); + } + auto len = reader.UnconsumedBufferLength(); if (args.MessageType() == SocketMessageType::Utf8) { reader.UnicodeEncoding(UnicodeEncoding::Utf8); From df553c6ab2f88d94d6a2934525144d9ed973ac90 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 13 Apr 2022 11:23:55 -0500 Subject: [PATCH 3/4] Consolidate catch clauses in MessageReceived --- .../WebSocketIntegrationTest.cpp | 56 ++++++++++--------- vnext/Shared/WinRTWebSocketResource.cpp | 39 ++++++------- 2 files changed, 46 insertions(+), 49 deletions(-) diff --git a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp index 95d5b8f0932..99ec4d72141 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp @@ -23,33 +23,6 @@ using Error = IWebSocketResource::Error; TEST_CLASS (WebSocketIntegrationTest) { - TEST_METHOD(AbruptServerDisconnectFailsWithSpecificMessage) - { - promise closedPromise; - auto ws = IWebSocketResource::Make(); - IWebSocketResource::Error error{{}, IWebSocketResource::ErrorType::None}; - CloseCode closeCode; - ws->SetOnClose([&closedPromise, &closeCode](CloseCode code, const string& reason) - { - closeCode = code; - closedPromise.set_value(); - }); - ws->SetOnError([&closedPromise, &error](IWebSocketResource::Error&& wsError) - { - error = std::move(wsError); - }); - - ws->Connect("ws://localhost:5555"); - - - closedPromise.get_future().wait(); - - //NOTE: This message is implementation-specific (WinRTWebSocketResource) - Assert::AreEqual({"[0x80072EFE] Underlying TCP connection suddenly terminated"}, error.Message); - Assert::AreEqual(static_cast(IWebSocketResource::ErrorType::Connection), static_cast(error.Type)); - Assert::AreEqual(static_cast(CloseCode::BadPayload), static_cast(closeCode)); - } - void SendReceiveCloseBase(bool isSecure) { auto server = make_shared(5556, isSecure); @@ -402,4 +375,33 @@ TEST_CLASS (WebSocketIntegrationTest) Assert::AreEqual({}, errorMessage); Assert::AreEqual(expected, result); } + + BEGIN_TEST_METHOD_ATTRIBUTE(AbruptDisconnectFailsWithSpecificMessage) + TEST_IGNORE() //TODO: Find a way to emulate abrupt disconnection using Test::WebSocketServer + END_TEST_METHOD_ATTRIBUTE() + TEST_METHOD(AbruptDisconnectFailsWithSpecificMessage) + { + promise closedPromise; + auto ws = IWebSocketResource::Make(); + IWebSocketResource::Error error{{}, IWebSocketResource::ErrorType::None}; + CloseCode closeCode; + ws->SetOnClose([&closedPromise, &closeCode](CloseCode code, const string& reason) + { + closeCode = code; + closedPromise.set_value(); + }); + ws->SetOnError([&closedPromise, &error](IWebSocketResource::Error&& wsError) + { + error = std::move(wsError); + }); + + ws->Connect("ws://localhost:5555"); + + closedPromise.get_future().wait(); + + //NOTE: This message is implementation-specific (WinRTWebSocketResource) + Assert::AreEqual({"[0x80072EFE] Underlying TCP connection suddenly terminated"}, error.Message); + Assert::AreEqual(static_cast(IWebSocketResource::ErrorType::Connection), static_cast(error.Type)); + Assert::AreEqual(static_cast(CloseCode::BadPayload), static_cast(closeCode)); + } }; diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index 2a4093d77a4..a3e7772b17a 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -296,27 +296,7 @@ void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, c [self = shared_from_this()](IWebSocket const &sender, IMessageWebSocketMessageReceivedEventArgs const &args) { try { string response; - IDataReader reader = nullptr; - try { - reader = args.GetDataReader(); - } catch (hresult_error const &e) { - string message; - ErrorType errorType; - - // See https://docs.microsoft.com/uwp/api/windows.networking.sockets.messagewebsocketmessagereceivedeventargs.getdatareader?view=winrt-19041#remarks - if (e.code() == WININET_E_CONNECTION_ABORTED) { - message = "[0x80072EFE] Underlying TCP connection suddenly terminated"; - errorType = ErrorType::Connection; - } else { - message = Utilities::HResultToString(e); - errorType = ErrorType::Receive; - } - - self->m_errorHandler({message, errorType}); - - return self->Close(CloseCode::BadPayload, message); - } - + IDataReader reader = args.GetDataReader(); auto len = reader.UnconsumedBufferLength(); if (args.MessageType() == SocketMessageType::Utf8) { reader.UnicodeEncoding(UnicodeEncoding::Utf8); @@ -336,7 +316,22 @@ void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, c } } catch (hresult_error const &e) { if (self->m_errorHandler) { - self->m_errorHandler({Utilities::HResultToString(e), ErrorType::Receive}); + string errorMessage; + ErrorType errorType; + // See + // https://docs.microsoft.com/uwp/api/windows.networking.sockets.messagewebsocketmessagereceivedeventargs.getdatareader?view=winrt-19041#remarks + if (e.code() == WININET_E_CONNECTION_ABORTED) { + errorMessage = "[0x80072EFE] Underlying TCP connection suddenly terminated"; + errorType = ErrorType::Connection; + self->m_errorHandler({errorMessage, errorType}); + + // Note: We are not clear whether all read-related errors should close the socket. + self->Close(CloseCode::BadPayload, std::move(errorMessage)); + } else { + errorMessage = Utilities::HResultToString(e); + errorType = ErrorType::Receive; + self->m_errorHandler({errorMessage, errorType}); + } } } }); From 3dd8a4aeef13a17d0ce0228e69bd6fb8e3bf0f48 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 13 Apr 2022 11:25:22 -0500 Subject: [PATCH 4/4] Change files --- ...ative-windows-11d58db5-55b8-4782-b499-1baaeb791c35.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/react-native-windows-11d58db5-55b8-4782-b499-1baaeb791c35.json diff --git a/change/react-native-windows-11d58db5-55b8-4782-b499-1baaeb791c35.json b/change/react-native-windows-11d58db5-55b8-4782-b499-1baaeb791c35.json new file mode 100644 index 00000000000..a2aa0be10f0 --- /dev/null +++ b/change/react-native-windows-11d58db5-55b8-4782-b499-1baaeb791c35.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Handle abrupt WebSocket connection interruption", + "packageName": "react-native-windows", + "email": "julio.rocha@microsoft.com", + "dependentChangeType": "patch" +}