From af33163a4781f18db5aaacd0ba0839a4236b83c2 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Tue, 21 Jul 2026 15:50:07 +0200 Subject: [PATCH 1/3] fix: harden settings error recovery --- .../Client/src/api/types.gen.ts | 1 + .../settings/settings-dashboard.element.ts | 28 +++++++++--- .../src/settings/settings-dashboard.test.ts | 37 +++++++++++++-- .../src/settings/settings-error.test.ts | 26 +++++++++++ .../Client/src/settings/settings-error.ts | 45 +++++++++++++++++++ .../src/settings/settings-model.test.ts | 1 + .../WebAnalyticsSettingsApiController.cs | 9 ++++ .../Models/AnalyticsModels.cs | 1 + .../WebAnalyticsSettingsApiControllerTests.cs | 1 + 9 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts create mode 100644 src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts diff --git a/src/TheBuilder.WebAnalytics/Client/src/api/types.gen.ts b/src/TheBuilder.WebAnalytics/Client/src/api/types.gen.ts index 8596cad..357e1b0 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/api/types.gen.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/api/types.gen.ts @@ -172,6 +172,7 @@ export type AnalyticsProviderTokenStatus = { }; export type AnalyticsSettingsResponse = { + packageVersion: string; enabled: boolean; providers: Array; providerTokens: Array; diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts index 4acf89e..b58ef55 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts @@ -21,6 +21,7 @@ import { createSettingsUpdate, validateConnection, validateEditableSettings } fr import { announceAnalyticsAvailability } from "../section/analytics-availability.js"; import { MOCK_SCENARIOS, type MockScenarioDefinition } from "./mock-scenarios.js"; import { identifierField, providerDescriptor, providerLogo } from "./provider-identity.js"; +import { settingsLoadError, type SettingsLoadError } from "./settings-error.js"; type NewConnection = | { kind: "provider"; descriptor: AnalyticsProviderDescriptor; hasAccessToken: boolean } @@ -35,6 +36,7 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle @state() private _showValidation = false; @state() private _testingKey?: string; @state() private _status?: { type: "success" | "error"; message: string }; + @state() private _loadError?: SettingsLoadError; @state() private _connectionStatuses: Record = {}; @state() private _showProviderPicker = false; @@ -46,17 +48,18 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle async #load(): Promise { this._loading = true; try { - const { data, error } = await WebAnalyticsService.settings(); + const { data, error, response } = await WebAnalyticsService.settings(); if (error || !data) { - this._status = { type: "error", message: "Analytics settings could not be loaded. Administrator access is required." }; + this._loadError = settingsLoadError(error, response?.status); return; } this._settings = data; this._dirty = false; this._showValidation = false; this._status = undefined; - } catch { - this._status = { type: "error", message: "Analytics settings could not be loaded. Administrator access is required." }; + this._loadError = undefined; + } catch (error) { + this._loadError = settingsLoadError(error); } finally { this._loading = false; } @@ -325,7 +328,15 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle render() { if (this._loading) return html``; if (!this._settings) return html` -

${this._status?.message}

Retry + `; const settings = this._settings; @@ -382,6 +393,8 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle ${this.#renderGeneralSettings()} ${this.#renderDevelopmentData()} +
Current installed version of Web Analytics: ${settings.packageVersion}
+ `; } @@ -396,6 +409,10 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle display: block; } form { max-width: var(--settings-column-max); margin-inline: auto; padding: var(--uui-size-layout-1) var(--settings-inline-gutter) calc(var(--uui-size-layout-1) + var(--uui-size-14) + var(--uui-size-space-4)); } + .load-error { box-sizing: border-box; margin-inline: auto; max-width: var(--settings-column-max); padding: var(--uui-size-layout-1) var(--settings-inline-gutter); } + .load-error-content { align-items: flex-start; display: flex; gap: var(--uui-size-space-3); margin-block-end: var(--uui-size-space-5); max-inline-size: 70ch; } + .load-error-content uui-icon { color: var(--uui-color-danger-standalone); flex: 0 0 auto; font-size: var(--uui-size-6); } + .load-error-content p { margin: 0; overflow-wrap: anywhere; text-wrap: pretty; } .section-heading { display: flex; align-items: center; justify-content: space-between; gap: var(--uui-size-layout-1); } .section-heading > div { min-inline-size: 0; } .settings-actions { @@ -453,6 +470,7 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle .field-with-help { display: grid; gap: var(--uui-size-space-1); } .field-help { color: var(--uui-color-text-alt); font-size: var(--uui-type-small-size); } .package-status { min-inline-size: 0; } + .package-version { color: var(--uui-color-text-alt); font-size: var(--uui-type-small-size); margin-block-start: var(--uui-size-layout-2); overflow-wrap: anywhere; } .visually-hidden { block-size: 1px; clip: rect(0 0 0 0); clip-path: inset(50%); inline-size: 1px; overflow: hidden; position: absolute; white-space: nowrap; } .section-heading { margin-bottom: var(--uui-size-space-4); } .provider-picker { background: var(--uui-color-surface-alt); border-block: 1px solid var(--uui-color-border); margin-block-end: var(--uui-size-space-5); padding: var(--uui-size-space-4) var(--uui-size-space-5) var(--uui-size-space-5); } diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts index d364dd0..bf58b60 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts @@ -20,6 +20,9 @@ import type { AnalyticsConnectionEditorElement } from "./connection-editor.eleme import "./settings-dashboard.element.js"; beforeEach(() => { + sdk.settings.mockReset(); + sdk.saveSettings.mockReset(); + sdk.testConnection.mockReset(); Element.prototype.scrollIntoView = vi.fn(); Object.defineProperty(navigator, "clipboard", { configurable: true, @@ -41,8 +44,9 @@ describe("analytics settings network recovery", () => { const dashboard = document.createElement("web-analytics-settings-dashboard") as WebAnalyticsSettingsDashboardElement; document.body.append(dashboard); - await vi.waitFor(() => expect(dashboard.shadowRoot?.querySelector("umb-empty-state")).not.toBeNull()); - expect(dashboard.shadowRoot?.textContent).toContain("Analytics settings could not be loaded."); + await vi.waitFor(() => expect(dashboard.shadowRoot?.querySelector(".load-error")).not.toBeNull()); + expect(dashboard.shadowRoot?.querySelector("uui-box")?.getAttribute("headline")).toBe("Could not reach the settings service"); + expect(dashboard.shadowRoot?.textContent).toContain("Check your connection and that Umbraco is running"); dashboard.shadowRoot?.querySelector('[label="Retry loading settings"]')?.click(); @@ -50,6 +54,30 @@ describe("analytics settings network recovery", () => { expect(sdk.settings).toHaveBeenCalledTimes(2); }); + it.each([ + [401, "Sign in again", "session has expired"], + [403, "Administrator access required", "Only administrators"], + [404, "Package files are out of sync", "Restart Umbraco"], + [503, "Settings service unavailable", "Umbraco logs"], + ])("shows a specific recovery for HTTP %s", async (status, headline, message) => { + sdk.settings.mockResolvedValueOnce(apiError(status)); + + const dashboard = document.createElement("web-analytics-settings-dashboard") as WebAnalyticsSettingsDashboardElement; + document.body.append(dashboard); + + await vi.waitFor(() => expect(dashboard.shadowRoot?.querySelector(".load-error")).not.toBeNull()); + expect(dashboard.shadowRoot?.querySelector("uui-box")?.getAttribute("headline")).toBe(headline); + expect(dashboard.shadowRoot?.textContent).toContain(message); + }); + + it("shows the installed package version below the settings", async () => { + const dashboard = document.createElement("web-analytics-settings-dashboard") as WebAnalyticsSettingsDashboardElement; + document.body.append(dashboard); + + await vi.waitFor(() => expect(dashboard.shadowRoot?.querySelector(".package-version")).not.toBeNull()); + expect(dashboard.shadowRoot?.querySelector(".package-version")?.textContent).toContain("Current installed version of Web Analytics: 0.3.0"); + }); + it.each([ ["an SDK error result", () => sdk.testConnection.mockResolvedValueOnce(apiError())], ["a rejected request", () => sdk.testConnection.mockRejectedValueOnce(new Error("Network unavailable"))], @@ -132,6 +160,7 @@ describe("analytics settings network recovery", () => { function settings(overrides: Partial = {}): AnalyticsSettingsResponse { return { + packageVersion: "0.3.0", enabled: true, providerTokens: [{ provider: "Vercel", hasAccessToken: false }, { provider: "Plausible", hasAccessToken: false }], canCreateMockConnections: false, @@ -359,6 +388,6 @@ function apiOk(data: T) { return { data, error: undefined, request: new Request("https://example.com"), response: new Response(null, { status: 200 }) }; } -function apiError() { - return { data: undefined, error: { message: "Request failed" }, request: new Request("https://example.com"), response: new Response(null, { status: 500 }) }; +function apiError(status = 500) { + return { data: undefined, error: { message: "Request failed" }, request: new Request("https://example.com"), response: new Response(null, { status }) }; } diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts new file mode 100644 index 0000000..0cb9396 --- /dev/null +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "vitest"; +import { settingsLoadError } from "./settings-error.js"; + +describe("settingsLoadError", () => { + it.each([ + [401, "Sign in again", "session has expired"], + [403, "Administrator access required", "Only administrators"], + [404, "Package files are out of sync", "server and package files use the same version"], + [429, "Too many requests", "Wait a moment"], + [503, "Settings service unavailable", "Umbraco logs"], + ])("describes HTTP %s with a useful recovery", (status, headline, message) => { + expect(settingsLoadError({}, status)).toEqual(expect.objectContaining({ headline })); + expect(settingsLoadError({}, status).message).toContain(message); + }); + + it("uses a network recovery message when no response exists", () => { + expect(settingsLoadError(new TypeError("Failed to fetch"))).toEqual({ + headline: "Could not reach the settings service", + message: "Check your connection and that Umbraco is running, then retry loading Web Analytics settings.", + }); + }); + + it("uses a status carried by an SDK error when no response is available", () => { + expect(settingsLoadError({ status: 403 }).headline).toBe("Administrator access required"); + }); +}); diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts new file mode 100644 index 0000000..2b9a485 --- /dev/null +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts @@ -0,0 +1,45 @@ +export interface SettingsLoadError { + headline: string; + message: string; +} + +export function settingsLoadError(error: unknown, responseStatus?: number): SettingsLoadError { + const errorStatus = typeof error === "object" && error !== null && "status" in error + ? Number(error.status) + : undefined; + const status = responseStatus || errorStatus; + + switch (status) { + case 401: + return { + headline: "Sign in again", + message: "Your backoffice session has expired. Sign in again, then retry loading Web Analytics settings.", + }; + case 403: + return { + headline: "Administrator access required", + message: "Only administrators can manage Web Analytics settings. Ask an administrator to open this page.", + }; + case 404: + return { + headline: "Package files are out of sync", + message: "The settings API was not found. Restart Umbraco after updating Web Analytics, then refresh the backoffice so the server and package files use the same version.", + }; + case 429: + return { + headline: "Too many requests", + message: "Web Analytics settings cannot be loaded right now. Wait a moment, then retry.", + }; + default: + if (status !== undefined && status >= 500) { + return { + headline: "Settings service unavailable", + message: "Umbraco could not load Web Analytics settings. Retry, and check the Umbraco logs if the problem continues.", + }; + } + return { + headline: "Could not reach the settings service", + message: "Check your connection and that Umbraco is running, then retry loading Web Analytics settings.", + }; + } +} diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-model.test.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-model.test.ts index 4460cdd..b5862b5 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-model.test.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-model.test.ts @@ -10,6 +10,7 @@ import { import { providerDescriptor } from "./provider-identity.js"; const settings = (): AnalyticsSettingsResponse => ({ + packageVersion: "0.3.0", enabled: true, providers: [ { diff --git a/src/TheBuilder.WebAnalytics/Controllers/WebAnalyticsSettingsApiController.cs b/src/TheBuilder.WebAnalytics/Controllers/WebAnalyticsSettingsApiController.cs index b7bcbb8..67525b6 100644 --- a/src/TheBuilder.WebAnalytics/Controllers/WebAnalyticsSettingsApiController.cs +++ b/src/TheBuilder.WebAnalytics/Controllers/WebAnalyticsSettingsApiController.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; +using System.Reflection; using Umbraco.Cms.Core.Security; using Umbraco.Extensions; using TheBuilder.WebAnalytics.Configuration; @@ -20,6 +21,13 @@ public sealed class WebAnalyticsSettingsApiController( IAnalyticsProviderClientResolver providerClients, IAnalyticsConnectionNameService projectNames) : WebAnalyticsApiControllerBase { + private static readonly string PackageVersion = + typeof(WebAnalyticsSettingsApiController).Assembly + .GetCustomAttribute()? + .InformationalVersion.Split('+')[0] + ?? typeof(WebAnalyticsSettingsApiController).Assembly.GetName().Version?.ToString(3) + ?? "unknown"; + [HttpGet("settings")] [ProducesResponseType(StatusCodes.Status200OK)] public async Task> Settings(CancellationToken cancellationToken) @@ -143,6 +151,7 @@ private async Task CreateResponseAsync(CancellationTo }); var responseConnections = await Task.WhenAll(responseTasks); return new AnalyticsSettingsResponse( + PackageVersion, settings.Enabled, AnalyticsProviderCatalog.Default.Definitions .Select(definition => definition.ToDescriptor()) diff --git a/src/TheBuilder.WebAnalytics/Models/AnalyticsModels.cs b/src/TheBuilder.WebAnalytics/Models/AnalyticsModels.cs index 68186e9..cd00137 100644 --- a/src/TheBuilder.WebAnalytics/Models/AnalyticsModels.cs +++ b/src/TheBuilder.WebAnalytics/Models/AnalyticsModels.cs @@ -169,6 +169,7 @@ public sealed record AnalyticsDocumentRoute( IReadOnlyList Warnings); public sealed record AnalyticsSettingsResponse( + string PackageVersion, bool Enabled, IReadOnlyList Providers, IReadOnlyList ProviderTokens, diff --git a/tests/TheBuilder.WebAnalytics.Tests/Controllers/WebAnalyticsSettingsApiControllerTests.cs b/tests/TheBuilder.WebAnalytics.Tests/Controllers/WebAnalyticsSettingsApiControllerTests.cs index f3dad8f..55b198c 100644 --- a/tests/TheBuilder.WebAnalytics.Tests/Controllers/WebAnalyticsSettingsApiControllerTests.cs +++ b/tests/TheBuilder.WebAnalytics.Tests/Controllers/WebAnalyticsSettingsApiControllerTests.cs @@ -51,6 +51,7 @@ public async Task Settings_preserve_mock_identity_and_report_runtime_availabilit var result = await controller.Settings(CancellationToken.None); var response = Assert.IsType(Assert.IsType(result.Result).Value); + Assert.Equal("0.1.0", response.PackageVersion); var connection = Assert.Single(response.Connections); Assert.Equal(MockAnalyticsScenario.Flags, connection.MockScenario); Assert.Equal(mockConnectionsEnabled, response.CanCreateMockConnections); From 4f80100a821ff53653b51cc847b2968be7784290 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Tue, 21 Jul 2026 16:16:43 +0200 Subject: [PATCH 2/3] fix: explain settings action failures --- .../settings/settings-dashboard.element.ts | 18 +++---- .../src/settings/settings-dashboard.test.ts | 52 ++++++++++++++++--- .../src/settings/settings-error.test.ts | 25 ++++++++- .../Client/src/settings/settings-error.ts | 44 ++++++++++++++-- 4 files changed, 119 insertions(+), 20 deletions(-) diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts index b58ef55..fecea7d 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts @@ -21,7 +21,7 @@ import { createSettingsUpdate, validateConnection, validateEditableSettings } fr import { announceAnalyticsAvailability } from "../section/analytics-availability.js"; import { MOCK_SCENARIOS, type MockScenarioDefinition } from "./mock-scenarios.js"; import { identifierField, providerDescriptor, providerLogo } from "./provider-identity.js"; -import { settingsLoadError, type SettingsLoadError } from "./settings-error.js"; +import { settingsActionError, settingsLoadError, type SettingsLoadError } from "./settings-error.js"; type NewConnection = | { kind: "provider"; descriptor: AnalyticsProviderDescriptor; hasAccessToken: boolean } @@ -158,17 +158,17 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle this._testingKey = key; this._connectionStatuses = { ...this._connectionStatuses, [key]: { type: "info", message: "Testing the saved connection…" } }; try { - const { data, error } = await WebAnalyticsService.testConnection({ path: { key } }); + const { data, error, response } = await WebAnalyticsService.testConnection({ path: { key } }); this._connectionStatuses = { ...this._connectionStatuses, [key]: error || !data - ? { type: "error", message: "The connection test could not be completed." } + ? { type: "error", message: settingsActionError("test", error, response?.status) } : { type: data.success ? "success" : "error", message: data.message }, }; - } catch { + } catch (error) { this._connectionStatuses = { ...this._connectionStatuses, - [key]: { type: "error", message: "The connection test could not be completed." }, + [key]: { type: "error", message: settingsActionError("test", error) }, }; } finally { this._testingKey = undefined; @@ -195,9 +195,9 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle this._status = undefined; const body: UpdateAnalyticsSettingsRequest = createSettingsUpdate(this._settings); try { - const { data, error } = await WebAnalyticsService.saveSettings({ body }); + const { data, error, response } = await WebAnalyticsService.saveSettings({ body }); if (error || !data) { - this._status = { type: "error", message: "Settings were not saved. Check the connection fields and mapping values." }; + this._status = { type: "error", message: settingsActionError("save", error, response?.status) }; return false; } this._settings = data; @@ -206,8 +206,8 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle announceAnalyticsAvailability(data.enabled); if (successMessage) this._status = { type: "success", message: successMessage }; return true; - } catch { - this._status = { type: "error", message: "Settings were not saved. Check the connection fields and mapping values." }; + } catch (error) { + this._status = { type: "error", message: settingsActionError("save", error) }; return false; } finally { this._saving = false; diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts index bf58b60..057ffae 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.test.ts @@ -79,9 +79,9 @@ describe("analytics settings network recovery", () => { }); it.each([ - ["an SDK error result", () => sdk.testConnection.mockResolvedValueOnce(apiError())], - ["a rejected request", () => sdk.testConnection.mockRejectedValueOnce(new Error("Network unavailable"))], - ])("shows a connection-local error and re-enables testing after %s", async (_description, arrangeResponse) => { + ["an SDK error result", () => sdk.testConnection.mockResolvedValueOnce(apiError()), "Umbraco logs"], + ["a rejected request", () => sdk.testConnection.mockRejectedValueOnce(new Error("Network unavailable")), "could not reach Umbraco"], + ])("shows a connection-local error and re-enables testing after %s", async (_description, arrangeResponse, message) => { arrangeResponse(); sdk.settings.mockResolvedValueOnce(apiOk(settings({ connections: [connection()] }))); @@ -92,7 +92,7 @@ describe("analytics settings network recovery", () => { const editor = dashboard.shadowRoot?.querySelector("web-analytics-connection-editor") as AnalyticsConnectionEditorElement; editor.dispatchEvent(new CustomEvent("test-connection", { bubbles: true, composed: true })); - await vi.waitFor(() => expect(editor.shadowRoot?.querySelector(".action-status")?.textContent).toContain("The connection test could not be completed.")); + await vi.waitFor(() => expect(editor.shadowRoot?.querySelector(".action-status")?.textContent).toContain(message)); expect(editor.testing).toBe(false); expect(editor.shadowRoot?.querySelector('[label="Test the saved connection."]')?.hasAttribute("disabled")).toBe(false); }); @@ -126,7 +126,7 @@ describe("analytics settings network recovery", () => { const form = dashboard.shadowRoot?.querySelector("form"); form?.dispatchEvent(new Event("submit", { bubbles: true, cancelable: true, composed: true })); - await vi.waitFor(() => expect(dashboard.shadowRoot?.textContent).toContain("Settings were not saved.")); + await vi.waitFor(() => expect(dashboard.shadowRoot?.textContent).toContain("could not reach Umbraco")); expect(dashboard.shadowRoot?.querySelector(".save-status")?.textContent).toContain("Unsaved changes"); expect(input!.value).toBe("31"); expect(dashboard.shadowRoot?.querySelector('[label="Save Web Analytics settings"]')?.hasAttribute("disabled")).toBe(false); @@ -152,10 +152,48 @@ describe("analytics settings network recovery", () => { await dashboard.updateComplete; dashboard.shadowRoot?.querySelector("form")?.dispatchEvent(new Event("submit", { bubbles: true, cancelable: true, composed: true })); - await vi.waitFor(() => expect(dashboard.shadowRoot?.textContent).toContain("Settings were not saved.")); + await vi.waitFor(() => expect(dashboard.shadowRoot?.textContent).toContain("could not be completed")); expect(dashboard.shadowRoot?.querySelector(".save-status")?.textContent).toContain("Unsaved changes"); expect(input!.value).toBe("31"); }); + + it.each([ + [401, "session has expired"], + [404, "server and package files use the same version"], + [503, "Umbraco logs"], + ])("preserves edits and explains save HTTP %s failures", async (status, message) => { + sdk.saveSettings.mockResolvedValueOnce(apiError(status)); + + const dashboard = document.createElement("web-analytics-settings-dashboard") as WebAnalyticsSettingsDashboardElement; + document.body.append(dashboard); + await vi.waitFor(() => expect(dashboard.shadowRoot?.querySelector("#default-range")).not.toBeNull()); + const input = dashboard.shadowRoot?.querySelector("#default-range"); + input!.value = "31"; + input!.dispatchEvent(new Event("input", { bubbles: true, composed: true })); + dashboard.shadowRoot?.querySelector("form")?.dispatchEvent(new Event("submit", { bubbles: true, cancelable: true, composed: true })); + + await vi.waitFor(() => expect(dashboard.shadowRoot?.querySelector(".status")?.textContent).toContain(message)); + expect(dashboard.shadowRoot?.querySelector(".save-status")?.textContent).toContain("Unsaved changes"); + expect(input!.value).toBe("31"); + }); + + it.each([ + [401, "session has expired"], + [404, "connection test API was not found"], + [503, "Umbraco logs"], + ])("explains connection-test HTTP %s failures", async (status, message) => { + sdk.testConnection.mockResolvedValueOnce(apiError(status)); + sdk.settings.mockResolvedValueOnce(apiOk(settings({ connections: [connection()] }))); + + const dashboard = document.createElement("web-analytics-settings-dashboard") as WebAnalyticsSettingsDashboardElement; + document.body.append(dashboard); + await vi.waitFor(() => expect(dashboard.shadowRoot?.querySelector("web-analytics-connection-editor")).not.toBeNull()); + const editor = dashboard.shadowRoot?.querySelector("web-analytics-connection-editor") as AnalyticsConnectionEditorElement; + editor.dispatchEvent(new CustomEvent("test-connection", { bubbles: true, composed: true })); + + await vi.waitFor(() => expect(editor.shadowRoot?.querySelector(".action-status")?.textContent).toContain(message)); + expect(editor.testing).toBe(false); + }); }); function settings(overrides: Partial = {}): AnalyticsSettingsResponse { @@ -295,6 +333,7 @@ describe("analytics settings onboarding", () => { it("marks new connections as using the configured shared token", async () => { sdk.settings.mockResolvedValue(apiOk({ + packageVersion: "0.3.0", enabled: true, providers: PROVIDERS, providerTokens: [{ provider: "Vercel", hasAccessToken: true }, { provider: "Plausible", hasAccessToken: false }], @@ -352,6 +391,7 @@ describe("analytics settings onboarding", () => { it("adds development mock scenarios as deterministic connections", async () => { sdk.settings.mockResolvedValue(apiOk({ + packageVersion: "0.3.0", enabled: true, providers: PROVIDERS, providerTokens: [{ provider: "Vercel", hasAccessToken: false }, { provider: "Plausible", hasAccessToken: false }], diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts index 0cb9396..ed1eef0 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { settingsLoadError } from "./settings-error.js"; +import { settingsActionError, settingsLoadError } from "./settings-error.js"; describe("settingsLoadError", () => { it.each([ @@ -24,3 +24,26 @@ describe("settingsLoadError", () => { expect(settingsLoadError({ status: 403 }).headline).toBe("Administrator access required"); }); }); + +describe("settingsActionError", () => { + it.each([ + ["save", 401, "session has expired"], + ["save", 404, "server and package files use the same version"], + ["save", 503, "Umbraco logs"], + ["test", 401, "retry the connection test"], + ["test", 404, "connection test API was not found"], + ["test", 503, "Umbraco logs"], + ] as const)("describes %s HTTP %s failures", (action, status, message) => { + expect(settingsActionError(action, {}, status)).toContain(message); + }); + + it("preserves a server validation detail when saving", () => { + expect(settingsActionError("save", { detail: "Cache duration must use hh:mm:ss." }, 400)) + .toBe("Cache duration must use hh:mm:ss."); + }); + + it("describes rejected requests as connection failures", () => { + expect(settingsActionError("save", new TypeError("Failed to fetch"))).toContain("could not reach Umbraco"); + expect(settingsActionError("test", new TypeError("Failed to fetch"))).toContain("could not reach Umbraco"); + }); +}); diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts index 2b9a485..2c4dad6 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts @@ -3,11 +3,10 @@ export interface SettingsLoadError { message: string; } +export type SettingsAction = "save" | "test"; + export function settingsLoadError(error: unknown, responseStatus?: number): SettingsLoadError { - const errorStatus = typeof error === "object" && error !== null && "status" in error - ? Number(error.status) - : undefined; - const status = responseStatus || errorStatus; + const status = requestStatus(error, responseStatus); switch (status) { case 401: @@ -43,3 +42,40 @@ export function settingsLoadError(error: unknown, responseStatus?: number): Sett }; } } + +export function settingsActionError(action: SettingsAction, error: unknown, responseStatus?: number): string { + const status = requestStatus(error, responseStatus); + const subject = action === "save" ? "Web Analytics settings" : "The connection test"; + + switch (status) { + case 400: + return action === "save" + ? problemDetail(error) ?? "Settings were not saved. Review the connection fields and mapping values." + : "The connection test request was invalid. Check the saved connection configuration."; + case 401: + return `Your backoffice session has expired. Sign in again, then ${action === "save" ? "save your preserved changes" : "retry the connection test"}.`; + case 403: + return `Administrator access is required to ${action === "save" ? "save Web Analytics settings" : "test analytics connections"}.`; + case 404: + return `${subject} API was not found. Restart Umbraco after updating Web Analytics, then refresh the backoffice so the server and package files use the same version.`; + case 429: + return `${subject} could not be completed because there were too many requests. Wait a moment, then retry.`; + default: + if (status !== undefined && status >= 500) { + return `${subject} could not be completed. Retry, and check the Umbraco logs if the problem continues.`; + } + return `${subject} could not reach Umbraco. Check your connection and that Umbraco is running, then retry.`; + } +} + +function requestStatus(error: unknown, responseStatus?: number): number | undefined { + const errorStatus = typeof error === "object" && error !== null && "status" in error + ? Number(error.status) + : undefined; + return responseStatus || errorStatus; +} + +function problemDetail(error: unknown): string | undefined { + if (typeof error !== "object" || error === null || !("detail" in error)) return undefined; + return typeof error.detail === "string" && error.detail.trim() ? error.detail : undefined; +} From 375228448d6e52472d6b8ea3491d7a38cfe797cc Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Tue, 21 Jul 2026 16:26:03 +0200 Subject: [PATCH 3/3] refactor: centralize API failure decoding --- .../Client/src/analytics/report-error.ts | 12 +-- .../Client/src/api/api-failure.test.ts | 24 +++++ .../Client/src/api/api-failure.ts | 24 +++++ .../settings/settings-dashboard.element.ts | 16 ++-- .../src/settings/settings-error.test.ts | 22 ++--- .../Client/src/settings/settings-error.ts | 91 ++++++------------- 6 files changed, 98 insertions(+), 91 deletions(-) create mode 100644 src/TheBuilder.WebAnalytics/Client/src/api/api-failure.test.ts create mode 100644 src/TheBuilder.WebAnalytics/Client/src/api/api-failure.ts diff --git a/src/TheBuilder.WebAnalytics/Client/src/analytics/report-error.ts b/src/TheBuilder.WebAnalytics/Client/src/analytics/report-error.ts index 1683dc0..795b665 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/analytics/report-error.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/analytics/report-error.ts @@ -1,15 +1,11 @@ +import { apiFailure } from "../api/api-failure.js"; + export function reportApiErrorMessage(error: unknown, status: number): string { - return reportErrorMessage(typeof error === "object" && error !== null - ? { ...error, status } - : { status }); + return reportErrorMessage(apiFailure(error, status)); } export function reportErrorMessage(error: unknown): string { - const problem = typeof error === "object" && error !== null - ? error as { code?: unknown; status?: unknown } - : undefined; - const code = typeof problem?.code === "string" ? problem.code : undefined; - const status = problem?.status === undefined ? undefined : Number(problem.status); + const { code, status } = apiFailure(error); switch (code) { case "invalid_credentials": diff --git a/src/TheBuilder.WebAnalytics/Client/src/api/api-failure.test.ts b/src/TheBuilder.WebAnalytics/Client/src/api/api-failure.test.ts new file mode 100644 index 0000000..a720a9c --- /dev/null +++ b/src/TheBuilder.WebAnalytics/Client/src/api/api-failure.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { apiFailure } from "./api-failure.js"; + +describe("apiFailure", () => { + it("normalizes supported problem fields", () => { + expect(apiFailure({ code: "invalid_query", detail: "Unsupported dimension", status: "400" })).toEqual({ + code: "invalid_query", + detail: "Unsupported dimension", + status: 400, + }); + }); + + it("prefers the HTTP response status", () => { + expect(apiFailure({ status: 400 }, 404).status).toBe(404); + }); + + it("ignores malformed problem fields", () => { + expect(apiFailure({ code: 42, detail: "", status: "unknown" })).toEqual({ + code: undefined, + detail: undefined, + status: undefined, + }); + }); +}); diff --git a/src/TheBuilder.WebAnalytics/Client/src/api/api-failure.ts b/src/TheBuilder.WebAnalytics/Client/src/api/api-failure.ts new file mode 100644 index 0000000..0e31259 --- /dev/null +++ b/src/TheBuilder.WebAnalytics/Client/src/api/api-failure.ts @@ -0,0 +1,24 @@ +export interface ApiFailure { + code?: string; + detail?: string; + status?: number; +} + +export function apiFailure(error: unknown, responseStatus?: number): ApiFailure { + const problem = typeof error === "object" && error !== null ? error as Record : undefined; + return { + code: stringProperty(problem, "code"), + detail: stringProperty(problem, "detail"), + status: responseStatus ?? numberProperty(problem, "status"), + }; +} + +function stringProperty(value: Record | undefined, property: string): string | undefined { + const candidate = value?.[property]; + return typeof candidate === "string" && candidate.trim() ? candidate : undefined; +} + +function numberProperty(value: Record | undefined, property: string): number | undefined { + const candidate = Number(value?.[property]); + return Number.isFinite(candidate) ? candidate : undefined; +} diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts index fecea7d..3d83289 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-dashboard.element.ts @@ -21,7 +21,7 @@ import { createSettingsUpdate, validateConnection, validateEditableSettings } fr import { announceAnalyticsAvailability } from "../section/analytics-availability.js"; import { MOCK_SCENARIOS, type MockScenarioDefinition } from "./mock-scenarios.js"; import { identifierField, providerDescriptor, providerLogo } from "./provider-identity.js"; -import { settingsActionError, settingsLoadError, type SettingsLoadError } from "./settings-error.js"; +import { settingsError, type SettingsError } from "./settings-error.js"; type NewConnection = | { kind: "provider"; descriptor: AnalyticsProviderDescriptor; hasAccessToken: boolean } @@ -36,7 +36,7 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle @state() private _showValidation = false; @state() private _testingKey?: string; @state() private _status?: { type: "success" | "error"; message: string }; - @state() private _loadError?: SettingsLoadError; + @state() private _loadError?: SettingsError; @state() private _connectionStatuses: Record = {}; @state() private _showProviderPicker = false; @@ -50,7 +50,7 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle try { const { data, error, response } = await WebAnalyticsService.settings(); if (error || !data) { - this._loadError = settingsLoadError(error, response?.status); + this._loadError = settingsError("load", error, response?.status); return; } this._settings = data; @@ -59,7 +59,7 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle this._status = undefined; this._loadError = undefined; } catch (error) { - this._loadError = settingsLoadError(error); + this._loadError = settingsError("load", error); } finally { this._loading = false; } @@ -162,13 +162,13 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle this._connectionStatuses = { ...this._connectionStatuses, [key]: error || !data - ? { type: "error", message: settingsActionError("test", error, response?.status) } + ? { type: "error", message: settingsError("test", error, response?.status).message } : { type: data.success ? "success" : "error", message: data.message }, }; } catch (error) { this._connectionStatuses = { ...this._connectionStatuses, - [key]: { type: "error", message: settingsActionError("test", error) }, + [key]: { type: "error", message: settingsError("test", error).message }, }; } finally { this._testingKey = undefined; @@ -197,7 +197,7 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle try { const { data, error, response } = await WebAnalyticsService.saveSettings({ body }); if (error || !data) { - this._status = { type: "error", message: settingsActionError("save", error, response?.status) }; + this._status = { type: "error", message: settingsError("save", error, response?.status).message }; return false; } this._settings = data; @@ -207,7 +207,7 @@ export class WebAnalyticsSettingsDashboardElement extends UmbElementMixin(LitEle if (successMessage) this._status = { type: "success", message: successMessage }; return true; } catch (error) { - this._status = { type: "error", message: settingsActionError("save", error) }; + this._status = { type: "error", message: settingsError("save", error).message }; return false; } finally { this._saving = false; diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts index ed1eef0..674a95b 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; -import { settingsActionError, settingsLoadError } from "./settings-error.js"; +import { settingsError } from "./settings-error.js"; -describe("settingsLoadError", () => { +describe("settingsError", () => { it.each([ [401, "Sign in again", "session has expired"], [403, "Administrator access required", "Only administrators"], @@ -9,23 +9,23 @@ describe("settingsLoadError", () => { [429, "Too many requests", "Wait a moment"], [503, "Settings service unavailable", "Umbraco logs"], ])("describes HTTP %s with a useful recovery", (status, headline, message) => { - expect(settingsLoadError({}, status)).toEqual(expect.objectContaining({ headline })); - expect(settingsLoadError({}, status).message).toContain(message); + expect(settingsError("load", {}, status)).toEqual(expect.objectContaining({ headline })); + expect(settingsError("load", {}, status).message).toContain(message); }); it("uses a network recovery message when no response exists", () => { - expect(settingsLoadError(new TypeError("Failed to fetch"))).toEqual({ + expect(settingsError("load", new TypeError("Failed to fetch"))).toEqual({ headline: "Could not reach the settings service", message: "Check your connection and that Umbraco is running, then retry loading Web Analytics settings.", }); }); it("uses a status carried by an SDK error when no response is available", () => { - expect(settingsLoadError({ status: 403 }).headline).toBe("Administrator access required"); + expect(settingsError("load", { status: 403 }).headline).toBe("Administrator access required"); }); }); -describe("settingsActionError", () => { +describe("settings actions", () => { it.each([ ["save", 401, "session has expired"], ["save", 404, "server and package files use the same version"], @@ -34,16 +34,16 @@ describe("settingsActionError", () => { ["test", 404, "connection test API was not found"], ["test", 503, "Umbraco logs"], ] as const)("describes %s HTTP %s failures", (action, status, message) => { - expect(settingsActionError(action, {}, status)).toContain(message); + expect(settingsError(action, {}, status).message).toContain(message); }); it("preserves a server validation detail when saving", () => { - expect(settingsActionError("save", { detail: "Cache duration must use hh:mm:ss." }, 400)) + expect(settingsError("save", { detail: "Cache duration must use hh:mm:ss." }, 400).message) .toBe("Cache duration must use hh:mm:ss."); }); it("describes rejected requests as connection failures", () => { - expect(settingsActionError("save", new TypeError("Failed to fetch"))).toContain("could not reach Umbraco"); - expect(settingsActionError("test", new TypeError("Failed to fetch"))).toContain("could not reach Umbraco"); + expect(settingsError("save", new TypeError("Failed to fetch")).message).toContain("could not reach Umbraco"); + expect(settingsError("test", new TypeError("Failed to fetch")).message).toContain("could not reach Umbraco"); }); }); diff --git a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts index 2c4dad6..d6e0cea 100644 --- a/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts +++ b/src/TheBuilder.WebAnalytics/Client/src/settings/settings-error.ts @@ -1,81 +1,44 @@ -export interface SettingsLoadError { +import { apiFailure } from "../api/api-failure.js"; + +export interface SettingsError { headline: string; message: string; } -export type SettingsAction = "save" | "test"; +export type SettingsOperation = "load" | "save" | "test"; -export function settingsLoadError(error: unknown, responseStatus?: number): SettingsLoadError { - const status = requestStatus(error, responseStatus); +export function settingsError(operation: SettingsOperation, error: unknown, responseStatus?: number): SettingsError { + const failure = apiFailure(error, responseStatus); + const retryAction = operation === "load" ? "retry loading Web Analytics settings" : operation === "save" ? "save your preserved changes" : "retry the connection test"; + const subject = operation === "test" ? "The connection test" : "Web Analytics settings"; - switch (status) { - case 401: - return { - headline: "Sign in again", - message: "Your backoffice session has expired. Sign in again, then retry loading Web Analytics settings.", - }; - case 403: - return { - headline: "Administrator access required", - message: "Only administrators can manage Web Analytics settings. Ask an administrator to open this page.", - }; - case 404: - return { - headline: "Package files are out of sync", - message: "The settings API was not found. Restart Umbraco after updating Web Analytics, then refresh the backoffice so the server and package files use the same version.", - }; - case 429: - return { - headline: "Too many requests", - message: "Web Analytics settings cannot be loaded right now. Wait a moment, then retry.", - }; - default: - if (status !== undefined && status >= 500) { - return { - headline: "Settings service unavailable", - message: "Umbraco could not load Web Analytics settings. Retry, and check the Umbraco logs if the problem continues.", - }; - } - return { - headline: "Could not reach the settings service", - message: "Check your connection and that Umbraco is running, then retry loading Web Analytics settings.", - }; - } -} - -export function settingsActionError(action: SettingsAction, error: unknown, responseStatus?: number): string { - const status = requestStatus(error, responseStatus); - const subject = action === "save" ? "Web Analytics settings" : "The connection test"; - - switch (status) { + switch (failure.status) { case 400: - return action === "save" - ? problemDetail(error) ?? "Settings were not saved. Review the connection fields and mapping values." - : "The connection test request was invalid. Check the saved connection configuration."; + return operation === "save" + ? { headline: "Review the settings", message: failure.detail ?? "Settings were not saved. Review the connection fields and mapping values." } + : operation === "test" + ? { headline: "Invalid connection test", message: "The connection test request was invalid. Check the saved connection configuration." } + : unavailable("Settings request rejected", "Umbraco rejected the settings request. Refresh the backoffice, then retry."); case 401: - return `Your backoffice session has expired. Sign in again, then ${action === "save" ? "save your preserved changes" : "retry the connection test"}.`; + return unavailable("Sign in again", `Your backoffice session has expired. Sign in again, then ${retryAction}.`); case 403: - return `Administrator access is required to ${action === "save" ? "save Web Analytics settings" : "test analytics connections"}.`; + return unavailable("Administrator access required", operation === "load" + ? "Only administrators can manage Web Analytics settings. Ask an administrator to open this page." + : `Administrator access is required to ${operation === "save" ? "save Web Analytics settings" : "test analytics connections"}.`); case 404: - return `${subject} API was not found. Restart Umbraco after updating Web Analytics, then refresh the backoffice so the server and package files use the same version.`; + return unavailable("Package files are out of sync", `${operation === "load" ? "The settings" : subject} API was not found. Restart Umbraco after updating Web Analytics, then refresh the backoffice so the server and package files use the same version.`); case 429: - return `${subject} could not be completed because there were too many requests. Wait a moment, then retry.`; + return unavailable("Too many requests", `${subject} ${operation === "load" ? "cannot be loaded" : "could not be completed"} right now. Wait a moment, then retry.`); default: - if (status !== undefined && status >= 500) { - return `${subject} could not be completed. Retry, and check the Umbraco logs if the problem continues.`; + if (failure.status !== undefined && failure.status >= 500) { + return unavailable("Settings service unavailable", `${subject} could not be ${operation === "load" ? "loaded" : "completed"}. Retry, and check the Umbraco logs if the problem continues.`); } - return `${subject} could not reach Umbraco. Check your connection and that Umbraco is running, then retry.`; + return operation === "load" + ? unavailable("Could not reach the settings service", "Check your connection and that Umbraco is running, then retry loading Web Analytics settings.") + : unavailable("Could not reach the settings service", `${subject} could not reach Umbraco. Check your connection and that Umbraco is running, then retry.`); } } -function requestStatus(error: unknown, responseStatus?: number): number | undefined { - const errorStatus = typeof error === "object" && error !== null && "status" in error - ? Number(error.status) - : undefined; - return responseStatus || errorStatus; -} - -function problemDetail(error: unknown): string | undefined { - if (typeof error !== "object" || error === null || !("detail" in error)) return undefined; - return typeof error.detail === "string" && error.detail.trim() ? error.detail : undefined; +function unavailable(headline: string, message: string): SettingsError { + return { headline, message }; }