From 8267d548334ab52265e8f38c6d6d63a0b806397a Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 31 Oct 2023 11:24:03 +0100 Subject: [PATCH 1/2] Make Playwright throw exceptions for 500s and test --- backend/Testing/Browser/Base/PageTest.cs | 33 ++++++++++++ .../Base/UnexpectedResponseException.cs | 23 +++++++++ backend/Testing/Browser/SandboxPageTests.cs | 51 ++++++++++++++++--- .../(unauthenticated)/sandbox/+page.svelte | 6 +++ 4 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 backend/Testing/Browser/Base/UnexpectedResponseException.cs diff --git a/backend/Testing/Browser/Base/PageTest.cs b/backend/Testing/Browser/Base/PageTest.cs index 7680f897f..846a5ad20 100644 --- a/backend/Testing/Browser/Base/PageTest.cs +++ b/backend/Testing/Browser/Base/PageTest.cs @@ -17,6 +17,11 @@ public class PageTest : IAsyncLifetime public IPage Page => _fixture.Page; public IBrowser Browser => _fixture.Browser; public IBrowserContext Context => _fixture.Context; + /// + /// Exceptions that are deferred until the end of the test, because they can't + /// be cleanly thrown in sub-threads. + /// + private List DeferredExceptions { get; } = new(); public PageTest() { @@ -26,6 +31,21 @@ public PageTest() public ILocatorAssertions Expect(ILocator locator) => Assertions.Expect(locator); public IPageAssertions Expect(IPage page) => Assertions.Expect(page); public IAPIResponseAssertions Expect(IAPIResponse response) => Assertions.Expect(response); + /// + /// Consumes a deferred exception that was "thrown" in a sub-thread, and returns it + /// or throws if no exception of the given type is found. + /// + public UnexpectedResponseException ExpectDeferredException() + { + var exception = DeferredExceptions.ShouldHaveSingleItem(); + DeferredExceptions.Clear(); + return exception; + } + + public void ExpectNoDeferredExceptions() + { + DeferredExceptions.ShouldBeEmpty(); + } public virtual async Task InitializeAsync() { @@ -39,6 +59,14 @@ await Context.Tracing.StartAsync(new() Sources = true }); } + + Context.Response += (_, response) => + { + if (response.Status >= (int)HttpStatusCode.InternalServerError) + { + DeferredExceptions.Add(new UnexpectedResponseException(response)); + } + }; } public virtual async Task DisposeAsync() @@ -52,6 +80,11 @@ public virtual async Task DisposeAsync() } await _fixture.DisposeAsync(); + + if (DeferredExceptions.Any()) + { + throw new AggregateException(DeferredExceptions); + } } static readonly HttpClient HttpClient = new HttpClient(); diff --git a/backend/Testing/Browser/Base/UnexpectedResponseException.cs b/backend/Testing/Browser/Base/UnexpectedResponseException.cs new file mode 100644 index 000000000..7e4db97ba --- /dev/null +++ b/backend/Testing/Browser/Base/UnexpectedResponseException.cs @@ -0,0 +1,23 @@ +using System.Text.RegularExpressions; +using Microsoft.Playwright; + +namespace Testing.Browser.Base; + +public partial class UnexpectedResponseException : SystemException +{ + public static string MaskUrl(string url) + { + return JwtRegex().Replace(url, "*****"); + } + + public UnexpectedResponseException(IResponse response) + : this(response.StatusText, response.Status, response.Url) + { + } + + public UnexpectedResponseException(string statusText, int statusCode, string url) + : base($"Unexpected response: {statusText} ({statusCode}). URL: {MaskUrl(url)}.") { } + + [GeneratedRegex("[A-Za-z0-9-_]{10,}\\.[A-Za-z0-9-_]{20,}\\.[A-Za-z0-9-_]{10,}")] + private static partial Regex JwtRegex(); +} diff --git a/backend/Testing/Browser/SandboxPageTests.cs b/backend/Testing/Browser/SandboxPageTests.cs index d2b7bf8d2..e0b0dbacc 100644 --- a/backend/Testing/Browser/SandboxPageTests.cs +++ b/backend/Testing/Browser/SandboxPageTests.cs @@ -1,4 +1,4 @@ -using Shouldly; +using Microsoft.Playwright; using Testing.Browser.Base; using Testing.Browser.Page; @@ -8,16 +8,51 @@ namespace Testing.Browser; public class SandboxPageTests : PageTest { [Fact] - public async Task Goto500Works() + public async Task CatchGoto500InSameTab() + { + + await new SandboxPage(Page).Goto(); + await Page.RunAndWaitForResponseAsync(async () => + { + await Page.GetByText("Goto 500 page").ClickAsync(); + }, "/api/testing/test500NoException"); + ExpectDeferredException(); + } + + [Fact] + public async Task CatchGoto500InNewTab() { await new SandboxPage(Page).Goto(); - var request = await Page.RunAndWaitForRequestFinishedAsync(async () => + await Context.RunAndWaitForPageAsync(async () => { - await Page.GetByText("goto 500 page").ClickAsync(); + await Page.GetByText("goto 500 new tab").ClickAsync(); }); - var response = await request.ResponseAsync(); - response.ShouldNotBeNull(); - response.Ok.ShouldBeFalse(); - response.Status.ShouldBe(500); + ExpectDeferredException(); + } + + [Fact(Skip = "Playwright doesn't catch the document load request of pages opened with Ctrl+Click")] + public async Task CatchGoto500InNewTabWithCtrl() + { + await new SandboxPage(Page).Goto(); + await Context.RunAndWaitForPageAsync(async () => + { + await Page.GetByText("Goto 500 page").ClickAsync(new() + { + Modifiers = new[] { KeyboardModifier.Control }, + }); + }); + ExpectDeferredException(); + } + + [Fact] + public async Task CatchFetch500() + { + await new SandboxPage(Page).Goto(); + await Page.RunAndWaitForResponseAsync(async () => + { + await Page.GetByText("Fetch 500").ClickAsync(); + }, "/api/testing/test500NoException"); + ExpectDeferredException(); + await Expect(Page.Locator(".modal-box.bg-error:has-text('Internal Server Error (500)')")).ToBeVisibleAsync(); } } diff --git a/frontend/src/routes/(unauthenticated)/sandbox/+page.svelte b/frontend/src/routes/(unauthenticated)/sandbox/+page.svelte index 0b84a412a..e4f87a48d 100644 --- a/frontend/src/routes/(unauthenticated)/sandbox/+page.svelte +++ b/frontend/src/routes/(unauthenticated)/sandbox/+page.svelte @@ -5,11 +5,17 @@ function uploadFinished(): void { alert('upload done!'); } + + async function fetch500(): Promise { + return fetch('/api/testing/test500NoException'); + }

Sandbox

Goto 500 page + Goto 500 new tab +
From def31a58407613170b295d95ef53411b866d940e Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 31 Oct 2023 11:34:47 +0100 Subject: [PATCH 2/2] Show error dialog for 500 featch responses --- frontend/src/hooks.client.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/src/hooks.client.ts b/frontend/src/hooks.client.ts index 5160b2bd3..30e638bd0 100644 --- a/frontend/src/hooks.client.ts +++ b/frontend/src/hooks.client.ts @@ -63,11 +63,18 @@ function shouldTryAutoReload(updateDetected: boolean): boolean { */ handleFetch(async ({ fetch, args }) => { const response = await traceFetch(() => fetch(...args)); + if (response.status === 401 && location.pathname !== '/login') { throw redirect(307, '/logout'); } + + if (response.status >= 500) { + throw new Error(`Unexpected response: ${response.statusText} (${response.status}). URL: ${response.url}.`); + } + if (response.headers.get('lexbox-refresh-jwt') == 'true') { await invalidate(USER_LOAD_KEY); } + return response; });