From 90d10fcaa1da261f6cca551f16f6cb89d06aa523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Tue, 11 Aug 2026 08:51:46 -0300 Subject: [PATCH 1/3] fix(transport): store expect errorDetails as serializable string .NET Framework Exception.Data rejects non-serializable JsonElement values. Storing expect ErrorDetails as raw JSON text prevents orphaned assertion callbacks and keeps the driver connection alive after failing Expect(). Fixes https://github.com/microsoft/playwright-dotnet/issues/3342 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Playwright.Tests/ConnectionTests.cs | 92 +++++++++++++++++++++++++ src/Playwright/Core/Frame.cs | 26 ++++--- src/Playwright/Transport/Connection.cs | 14 +++- 3 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 src/Playwright.Tests/ConnectionTests.cs diff --git a/src/Playwright.Tests/ConnectionTests.cs b/src/Playwright.Tests/ConnectionTests.cs new file mode 100644 index 000000000..166c03559 --- /dev/null +++ b/src/Playwright.Tests/ConnectionTests.cs @@ -0,0 +1,92 @@ +/* + * MIT License + * + * Copyright (c) Microsoft Corporation. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and / or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +using System.Text.Json; +using Microsoft.Playwright.Transport; + +namespace Microsoft.Playwright.Tests; + +/// +/// Regression coverage for expect errorDetails attached to Exception.Data. +/// On .NET Framework, Exception.Data rejects non-serializable values (e.g. JsonElement), +/// which used to orphan the callback and kill the driver connection (issue #3342). +/// +public class ConnectionTests +{ + [Test] + public async Task Dispatch_ExpectErrorDetails_MustBeSerializableForExceptionData() + { + var connection = new Connection(); + var errorDetailsJson = "{\"received\":{\"value\":{\"v\":\"n\",\"n\":0}},\"customErrorMessage\":\"nope\"}"; + using var errorDetailsDocument = JsonDocument.Parse(errorDetailsJson); + var errorDetails = errorDetailsDocument.RootElement.Clone(); + + connection.OnMessage = (message, _) => + { + var dict = (Dictionary)message; + var id = (int)dict["id"]; + connection.Dispatch(new PlaywrightServerMessage + { + Id = id, + Error = new ErrorEntry + { + Error = new PlaywrightServerError + { + Message = "Error: expect failed", + Name = "Error", + }, + }, + ErrorDetails = errorDetails, + Log = new[] { "waiting for locator" }, + }); + return Task.CompletedTask; + }; + + var sendTask = connection.SendMessageToServerAsync( + null, + "expect", + new Dictionary { ["selector"] = "#missing" }); + + var completed = await Task.WhenAny(sendTask, Task.Delay(5000)); + Assert.AreSame(sendTask, completed, "expect error callback must complete (must not hang after Dispatch)"); + + var exception = await PlaywrightAssert.ThrowsAsync(() => sendTask); + StringAssert.Contains("expect failed", exception.Message); + + Assert.True(exception.Data.Contains(Connection.ErrorDetailsDataKey)); + var detailsValue = exception.Data[Connection.ErrorDetailsDataKey]; + // .NET Framework Exception.Data requires serializable values. JsonElement is not serializable. + Assert.That(detailsValue, Is.Null.Or.TypeOf(), + "ErrorDetails must be stored as a serializable string, not JsonElement"); + + if (detailsValue is string detailsText) + { + StringAssert.Contains("received", detailsText); + StringAssert.Contains("customErrorMessage", detailsText); + } + + Assert.True(exception.Data.Contains(Connection.LogDataKey)); + Assert.That(exception.Data[Connection.LogDataKey], Is.Null.Or.TypeOf()); + } +} diff --git a/src/Playwright/Core/Frame.cs b/src/Playwright/Core/Frame.cs index 4b48def53..19891796a 100644 --- a/src/Playwright/Core/Frame.cs +++ b/src/Playwright/Core/Frame.cs @@ -1013,21 +1013,27 @@ await SendMessageToServerAsync( Matches = options.IsNot, Log = (e.Data[Connection.LogDataKey] as string[]) ?? Array.Empty(), }; - if (e.Data[Connection.ErrorDetailsDataKey] is JsonElement details && details.ValueKind == JsonValueKind.Object) + // ErrorDetails is stored as raw JSON text for .NET Framework Exception.Data compatibility. + if (e.Data[Connection.ErrorDetailsDataKey] is string detailsJson && !string.IsNullOrEmpty(detailsJson)) { - if (details.TryGetProperty("customErrorMessage", out var customErrorMessage) && customErrorMessage.ValueKind == JsonValueKind.String) + using var detailsDocument = JsonDocument.Parse(detailsJson); + var details = detailsDocument.RootElement; + if (details.ValueKind == JsonValueKind.Object) { - result.ErrorMessage = "Error: " + customErrorMessage.GetString(); - } - if (details.TryGetProperty("received", out var received) && received.ValueKind == JsonValueKind.Object) - { - if (received.TryGetProperty("value", out var receivedValue)) + if (details.TryGetProperty("customErrorMessage", out var customErrorMessage) && customErrorMessage.ValueKind == JsonValueKind.String) { - result.Received = ScriptsHelper.ParseEvaluateResult(receivedValue); + result.ErrorMessage = "Error: " + customErrorMessage.GetString(); } - if (received.TryGetProperty("ariaSnapshot", out var ariaSnapshot) && ariaSnapshot.ValueKind == JsonValueKind.String) + if (details.TryGetProperty("received", out var received) && received.ValueKind == JsonValueKind.Object) { - result.ReceivedAriaSnapshot = ariaSnapshot.GetString(); + if (received.TryGetProperty("value", out var receivedValue)) + { + result.Received = ScriptsHelper.ParseEvaluateResult(receivedValue); + } + if (received.TryGetProperty("ariaSnapshot", out var ariaSnapshot) && ariaSnapshot.ValueKind == JsonValueKind.String) + { + result.ReceivedAriaSnapshot = ariaSnapshot.GetString(); + } } } } diff --git a/src/Playwright/Transport/Connection.cs b/src/Playwright/Transport/Connection.cs index 3471ba9e4..b317a07a9 100644 --- a/src/Playwright/Transport/Connection.cs +++ b/src/Playwright/Transport/Connection.cs @@ -286,8 +286,18 @@ internal void Dispatch(PlaywrightServerMessage message) if (message.Error != null && message.Result == null) { var exception = ParseException(message.Error.Error, FormatCallLog(message.Log)); - exception.Data[ErrorDetailsDataKey] = message.ErrorDetails; - exception.Data[LogDataKey] = message.Log; + // Exception.Data values must be serializable on .NET Framework. + // Store ErrorDetails as raw JSON text (not JsonElement) so expect + // failures cannot throw here, orphan the callback, and kill the connection. + try + { + exception.Data[ErrorDetailsDataKey] = message.ErrorDetails?.GetRawText(); + exception.Data[LogDataKey] = message.Log; + } + catch + { + // Best-effort enrichment only; completing the callback is mandatory. + } callback.TaskCompletionSource.TrySetException(exception); } else From 64d7c135d373721dd316ae795dd6d18e57a434a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Wed, 12 Aug 2026 11:28:42 -0300 Subject: [PATCH 2/3] fix(transport): drop try/catch and unit test per review Keep serializable ErrorDetails string storage only. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Playwright.Tests/ConnectionTests.cs | 92 ------------------------- src/Playwright/Transport/Connection.cs | 14 +--- 2 files changed, 3 insertions(+), 103 deletions(-) delete mode 100644 src/Playwright.Tests/ConnectionTests.cs diff --git a/src/Playwright.Tests/ConnectionTests.cs b/src/Playwright.Tests/ConnectionTests.cs deleted file mode 100644 index 166c03559..000000000 --- a/src/Playwright.Tests/ConnectionTests.cs +++ /dev/null @@ -1,92 +0,0 @@ -/* - * MIT License - * - * Copyright (c) Microsoft Corporation. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and / or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -using System.Text.Json; -using Microsoft.Playwright.Transport; - -namespace Microsoft.Playwright.Tests; - -/// -/// Regression coverage for expect errorDetails attached to Exception.Data. -/// On .NET Framework, Exception.Data rejects non-serializable values (e.g. JsonElement), -/// which used to orphan the callback and kill the driver connection (issue #3342). -/// -public class ConnectionTests -{ - [Test] - public async Task Dispatch_ExpectErrorDetails_MustBeSerializableForExceptionData() - { - var connection = new Connection(); - var errorDetailsJson = "{\"received\":{\"value\":{\"v\":\"n\",\"n\":0}},\"customErrorMessage\":\"nope\"}"; - using var errorDetailsDocument = JsonDocument.Parse(errorDetailsJson); - var errorDetails = errorDetailsDocument.RootElement.Clone(); - - connection.OnMessage = (message, _) => - { - var dict = (Dictionary)message; - var id = (int)dict["id"]; - connection.Dispatch(new PlaywrightServerMessage - { - Id = id, - Error = new ErrorEntry - { - Error = new PlaywrightServerError - { - Message = "Error: expect failed", - Name = "Error", - }, - }, - ErrorDetails = errorDetails, - Log = new[] { "waiting for locator" }, - }); - return Task.CompletedTask; - }; - - var sendTask = connection.SendMessageToServerAsync( - null, - "expect", - new Dictionary { ["selector"] = "#missing" }); - - var completed = await Task.WhenAny(sendTask, Task.Delay(5000)); - Assert.AreSame(sendTask, completed, "expect error callback must complete (must not hang after Dispatch)"); - - var exception = await PlaywrightAssert.ThrowsAsync(() => sendTask); - StringAssert.Contains("expect failed", exception.Message); - - Assert.True(exception.Data.Contains(Connection.ErrorDetailsDataKey)); - var detailsValue = exception.Data[Connection.ErrorDetailsDataKey]; - // .NET Framework Exception.Data requires serializable values. JsonElement is not serializable. - Assert.That(detailsValue, Is.Null.Or.TypeOf(), - "ErrorDetails must be stored as a serializable string, not JsonElement"); - - if (detailsValue is string detailsText) - { - StringAssert.Contains("received", detailsText); - StringAssert.Contains("customErrorMessage", detailsText); - } - - Assert.True(exception.Data.Contains(Connection.LogDataKey)); - Assert.That(exception.Data[Connection.LogDataKey], Is.Null.Or.TypeOf()); - } -} diff --git a/src/Playwright/Transport/Connection.cs b/src/Playwright/Transport/Connection.cs index b317a07a9..a986f2879 100644 --- a/src/Playwright/Transport/Connection.cs +++ b/src/Playwright/Transport/Connection.cs @@ -287,17 +287,9 @@ internal void Dispatch(PlaywrightServerMessage message) { var exception = ParseException(message.Error.Error, FormatCallLog(message.Log)); // Exception.Data values must be serializable on .NET Framework. - // Store ErrorDetails as raw JSON text (not JsonElement) so expect - // failures cannot throw here, orphan the callback, and kill the connection. - try - { - exception.Data[ErrorDetailsDataKey] = message.ErrorDetails?.GetRawText(); - exception.Data[LogDataKey] = message.Log; - } - catch - { - // Best-effort enrichment only; completing the callback is mandatory. - } + // Store ErrorDetails as raw JSON text (not JsonElement). + exception.Data[ErrorDetailsDataKey] = message.ErrorDetails?.GetRawText(); + exception.Data[LogDataKey] = message.Log; callback.TaskCompletionSource.TrySetException(exception); } else From d167169406716504d2806a84c4552587ab4fcac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Wed, 12 Aug 2026 11:58:43 -0300 Subject: [PATCH 3/3] Apply suggestion from @kblok --- src/Playwright/Transport/Connection.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Playwright/Transport/Connection.cs b/src/Playwright/Transport/Connection.cs index a986f2879..b13fb5721 100644 --- a/src/Playwright/Transport/Connection.cs +++ b/src/Playwright/Transport/Connection.cs @@ -286,8 +286,6 @@ internal void Dispatch(PlaywrightServerMessage message) if (message.Error != null && message.Result == null) { var exception = ParseException(message.Error.Error, FormatCallLog(message.Log)); - // Exception.Data values must be serializable on .NET Framework. - // Store ErrorDetails as raw JSON text (not JsonElement). exception.Data[ErrorDetailsDataKey] = message.ErrorDetails?.GetRawText(); exception.Data[LogDataKey] = message.Log; callback.TaskCompletionSource.TrySetException(exception);