From 0e3cbb3cf2d6f85bbaf4ce9137ab061b3e8121a4 Mon Sep 17 00:00:00 2001 From: James Brunton Date: Tue, 9 Jun 2026 09:34:02 +0100 Subject: [PATCH] Explicitly test for console warnings & errors (#6502) # Description of Changes Disallow warnings and errors from being thrown in the browser console during tests unless explicitly expected in the test. Also adds a Playwright test to prod around some main UI areas and checks that no warnings/errors have been thrown. --- .../core/contexts/AppConfigContext.test.tsx | 6 + .../src/core/contexts/AppConfigContext.tsx | 6 +- .../src/core/contexts/FolderContext.test.tsx | 7 +- .../src/core/contexts/FolderContext.tsx | 2 +- .../tools/shared/toolOperationHelpers.ts | 2 +- .../editor/src/core/hooks/useScarfTracking.ts | 3 - .../indexedDBManager.migration.test.ts | 7 + frontend/editor/src/core/setupTests.ts | 3 + .../tests/convert/ConvertIntegration.test.tsx | 10 ++ .../ConvertSmartDetectionIntegration.test.tsx | 16 ++ .../src/core/tests/failOnConsole.test.ts | 78 +++++++++ .../editor/src/core/tests/failOnConsole.ts | 152 ++++++++++++++++++ .../core/tests/stubbed/console-clean.spec.ts | 114 +++++++++++++ .../core/utils/automationConverter.test.ts | 2 + .../editor/src/core/utils/scarfTracking.ts | 24 ++- .../editor/src/core/utils/toolSynonyms.ts | 26 +-- .../hooks/useDesktopUpdatePopup.test.ts | 25 ++- .../desktop/services/apiClientSetup.test.ts | 4 + .../proprietary/auth/springAuthClient.test.ts | 28 ++++ .../src/proprietary/auth/springAuthClient.ts | 15 +- .../stripeCheckout/stages/PaymentStage.tsx | 12 +- .../proprietary/routes/AuthCallback.test.tsx | 4 + .../src/proprietary/routes/AuthCallback.tsx | 135 ++-------------- .../src/proprietary/routes/Login.test.tsx | 56 ++++--- .../editor/src/proprietary/routes/Login.tsx | 6 - .../components/shared/StripeCheckoutSaas.tsx | 9 +- .../src/saas/services/apiClient.test.ts | 2 + frontend/editor/src/saas/setupTests.ts | 3 + 28 files changed, 544 insertions(+), 213 deletions(-) create mode 100644 frontend/editor/src/core/tests/failOnConsole.test.ts create mode 100644 frontend/editor/src/core/tests/failOnConsole.ts create mode 100644 frontend/editor/src/core/tests/stubbed/console-clean.spec.ts diff --git a/frontend/editor/src/core/contexts/AppConfigContext.test.tsx b/frontend/editor/src/core/contexts/AppConfigContext.test.tsx index eb630b771..5e512ec69 100644 --- a/frontend/editor/src/core/contexts/AppConfigContext.test.tsx +++ b/frontend/editor/src/core/contexts/AppConfigContext.test.tsx @@ -5,6 +5,7 @@ import { useAppConfig, } from "@app/contexts/AppConfigContext"; import apiClient from "@app/services/apiClient"; +import { allowConsole, expectConsole } from "@app/tests/failOnConsole"; import { ReactNode } from "react"; // Mock apiClient @@ -92,6 +93,7 @@ describe("AppConfigContext", () => { }); it("should handle network errors", async () => { + expectConsole.error(/\[AppConfig\] Failed to fetch app config/); const errorMessage = "Network error occurred"; const mockError = new Error(errorMessage); // Network errors don't have response property @@ -249,6 +251,10 @@ describe("AppConfigContext", () => { }); it("should handle initial config prop", async () => { + // apiClient.get isn't mocked here, so the implicit "fetch fails" path + // exhausts retries and logs. Test only asserts the API was called, so + // the failure log is incidental. + allowConsole.error(/\[AppConfig\] Failed to fetch app config/); const initialConfig = { enableLogin: false, appNameNavbar: "Initial App", diff --git a/frontend/editor/src/core/contexts/AppConfigContext.tsx b/frontend/editor/src/core/contexts/AppConfigContext.tsx index 763534554..3672f6cdf 100644 --- a/frontend/editor/src/core/contexts/AppConfigContext.tsx +++ b/frontend/editor/src/core/contexts/AppConfigContext.tsx @@ -107,12 +107,10 @@ export const AppConfigProvider: React.FC = ({ if (attempt > 0) { const delay = initialDelay * Math.pow(2, attempt - 1); - console.log( + console.debug( `[AppConfig] Retry attempt ${attempt}/${maxRetries} after ${delay}ms delay...`, ); await sleep(delay); - } else { - console.log("[AppConfig] Fetching app config..."); } // apiClient automatically adds JWT header if available via interceptors @@ -165,7 +163,7 @@ export const AppConfigProvider: React.FC = ({ (!status || status >= 500) && attempt < maxRetries; if (shouldRetry) { - console.warn( + console.debug( `[AppConfig] Attempt ${attempt + 1} failed (status ${status || "network error"}):`, err.message, "- will retry...", diff --git a/frontend/editor/src/core/contexts/FolderContext.test.tsx b/frontend/editor/src/core/contexts/FolderContext.test.tsx index 171dec820..861ddd331 100644 --- a/frontend/editor/src/core/contexts/FolderContext.test.tsx +++ b/frontend/editor/src/core/contexts/FolderContext.test.tsx @@ -3,6 +3,7 @@ import { describe, expect, test, vi, beforeEach } from "vitest"; import { render, screen, waitFor, act } from "@testing-library/react"; import { FolderProvider, useFolders } from "@app/contexts/FolderContext"; +import { expectConsole } from "@app/tests/failOnConsole"; /** * Regression test for the sync-banner 4xx gating fix in commit c38b646c5. @@ -122,6 +123,9 @@ describe("FolderContext sync-banner gating", () => { }); test("500 (server error) DOES surface a banner", async () => { + // Surfacing the banner also logs a warning - that's the contract this branch + // tests for. + expectConsole.warn(/\[FolderContext\] pullFromServer failed/); mockList.mockRejectedValue(axiosError(500, "internal")); await renderAndWaitForPull(); expect(screen.getByTestId("error").textContent).toContain( @@ -133,7 +137,8 @@ describe("FolderContext sync-banner gating", () => { test("network error (no response) DOES surface a banner", async () => { // axios on a network failure rejects with an Error that has NO // `.response` property - that's the "status === undefined" branch - // the gate explicitly covers. + // the gate explicitly covers. Surfacing the banner also logs a warning. + expectConsole.warn(/\[FolderContext\] pullFromServer failed/); mockList.mockRejectedValue(new Error("ECONNREFUSED")); await renderAndWaitForPull(); expect(screen.getByTestId("error").textContent).toContain( diff --git a/frontend/editor/src/core/contexts/FolderContext.tsx b/frontend/editor/src/core/contexts/FolderContext.tsx index 0f421af1d..9c69fd0cf 100644 --- a/frontend/editor/src/core/contexts/FolderContext.tsx +++ b/frontend/editor/src/core/contexts/FolderContext.tsx @@ -272,7 +272,6 @@ export function FolderProvider({ children }: FolderProviderProps) { if (mountedRef.current) setServerReachable(false); return { ok: false, reason: "endpoint-missing" }; } - console.warn("[FolderContext] pullFromServer failed", err); if (mountedRef.current) { setServerReachable(false); // Only surface a banner when this is a server-side outage or @@ -284,6 +283,7 @@ export function FolderProvider({ children }: FolderProviderProps) { // manager. Folder-mutation buttons get individual disabled // tooltips via `serverReachable`, which is enough signal. if (status === undefined || status >= 500) { + console.warn("[FolderContext] pullFromServer failed", err); setError(`Folder sync failed: ${formatServerError(err)}`); } } diff --git a/frontend/editor/src/core/hooks/tools/shared/toolOperationHelpers.ts b/frontend/editor/src/core/hooks/tools/shared/toolOperationHelpers.ts index 37d52eacb..baaef647c 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolOperationHelpers.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolOperationHelpers.ts @@ -27,7 +27,7 @@ export function buildInputTracking( inputFileIds.push(fileId); inputStirlingFileStubs.push(record); } else { - console.warn(`No file stub found for file: ${file.name}`); + console.debug(`No file stub found for file: ${file.name}`); inputFileIds.push(fileId); inputStirlingFileStubs.push(createNewStirlingFileStub(file, fileId)); } diff --git a/frontend/editor/src/core/hooks/useScarfTracking.ts b/frontend/editor/src/core/hooks/useScarfTracking.ts index 27eed4b97..7c98e80c6 100644 --- a/frontend/editor/src/core/hooks/useScarfTracking.ts +++ b/frontend/editor/src/core/hooks/useScarfTracking.ts @@ -29,9 +29,6 @@ export function useScarfTracking() { // Listen to cookie consent changes and auto-fire pixel when consent is granted useEffect(() => { const handleConsentChange = () => { - console.warn( - "[useScarfTracking] Consent changed, checking scarf service acceptance", - ); if (isServiceAccepted("scarf", "analytics")) { firePixel(window.location.pathname); } diff --git a/frontend/editor/src/core/services/indexedDBManager.migration.test.ts b/frontend/editor/src/core/services/indexedDBManager.migration.test.ts index aecb75484..0a5a4b982 100644 --- a/frontend/editor/src/core/services/indexedDBManager.migration.test.ts +++ b/frontend/editor/src/core/services/indexedDBManager.migration.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test, beforeEach } from "vitest"; import "fake-indexeddb/auto"; +import { expectConsole } from "@app/tests/failOnConsole"; import { DATABASE_CONFIGS, @@ -363,6 +364,9 @@ describe("IndexedDB migration (FILES store)", () => { }); test("SaaS v6 database is force-deleted (data lost, schema reset to v9)", async () => { + // The force-delete warn IS the contract under test; production fires it + // to surface why data was wiped. + expectConsole.warn(/Deleting corrupt SaaS v6/); await seedSaasDatabase(6, ["v6-corrupt-file"]); await indexedDBManager.openDatabase(DATABASE_CONFIGS.FILES); @@ -381,6 +385,9 @@ describe("IndexedDB migration (FILES store)", () => { }); test("SaaS v7 database is force-deleted (data lost, schema reset to v9)", async () => { + // The force-delete warn IS the contract under test; production fires it + // to surface why data was wiped. + expectConsole.warn(/Deleting corrupt SaaS v7/); await seedSaasDatabase(7, ["v7-corrupt-file"]); await indexedDBManager.openDatabase(DATABASE_CONFIGS.FILES); diff --git a/frontend/editor/src/core/setupTests.ts b/frontend/editor/src/core/setupTests.ts index bd6e08325..57bcaa76a 100644 --- a/frontend/editor/src/core/setupTests.ts +++ b/frontend/editor/src/core/setupTests.ts @@ -1,5 +1,8 @@ import "@testing-library/jest-dom"; import { vi } from "vitest"; +import { installFailOnConsole } from "@app/tests/failOnConsole"; + +installFailOnConsole(); // Mock localStorage for tests class LocalStorageMock implements Storage { diff --git a/frontend/editor/src/core/tests/convert/ConvertIntegration.test.tsx b/frontend/editor/src/core/tests/convert/ConvertIntegration.test.tsx index 9ac5b7c28..e0045584f 100644 --- a/frontend/editor/src/core/tests/convert/ConvertIntegration.test.tsx +++ b/frontend/editor/src/core/tests/convert/ConvertIntegration.test.tsx @@ -29,6 +29,8 @@ import { PreferencesProvider } from "@app/contexts/PreferencesContext"; import { I18nextProvider } from "react-i18next"; import i18n from "@app/i18n/config"; import { createTestStirlingFile } from "@app/tests/utils/testFileHelpers"; +import { expectConsole } from "@app/tests/failOnConsole"; +import { fileStorage } from "@app/services/fileStorage"; import { StirlingFile } from "@app/types/fileContext"; import { MantineProvider } from "@mantine/core"; @@ -78,6 +80,7 @@ vi.mock("../../services/fileStorage", () => ({ thumbnail: thumbnail, }); }), + storeStirlingFile: vi.fn().mockResolvedValue(undefined), getAllFileMetadata: vi.fn().mockResolvedValue([]), cleanup: vi.fn().mockResolvedValue(undefined), }, @@ -209,6 +212,10 @@ describe("Convert Tool Integration Tests", () => { expect(result.current.downloadFilename).toBe("test.png"); expect(result.current.isLoading).toBe(false); expect(result.current.errorMessage).toBe(null); + + // The output file must be persisted via fileStorage.storeStirlingFile + // so downstream tools see it in the registry. + expect(fileStorage.storeStirlingFile).toHaveBeenCalled(); }); test("should handle API error responses correctly", async () => { @@ -571,6 +578,9 @@ describe("Convert Tool Integration Tests", () => { describe("File Upload Integration", () => { test("should handle multiple file uploads correctly", async () => { + // Test mocks only one apiClient.post response; the second file hits an + // undefined response and production warns about the conversion failure. + expectConsole.warn(/Failed to convert file test2\.pdf/); const mockBlob = new Blob(["zip-content"], { type: "application/zip" }); (mockedApiClient.post as Mock).mockResolvedValueOnce({ data: mockBlob }); diff --git a/frontend/editor/src/core/tests/convert/ConvertSmartDetectionIntegration.test.tsx b/frontend/editor/src/core/tests/convert/ConvertSmartDetectionIntegration.test.tsx index af5b99172..cfb7a2dd9 100644 --- a/frontend/editor/src/core/tests/convert/ConvertSmartDetectionIntegration.test.tsx +++ b/frontend/editor/src/core/tests/convert/ConvertSmartDetectionIntegration.test.tsx @@ -28,6 +28,8 @@ import { createTestStirlingFile, createTestFilesWithId, } from "@app/tests/utils/testFileHelpers"; +import { allowConsole, expectConsole } from "@app/tests/failOnConsole"; +import { fileStorage } from "@app/services/fileStorage"; import { MantineProvider } from "@mantine/core"; // Mock axios (for static methods like CancelToken, isCancel) @@ -76,6 +78,7 @@ vi.mock("../../services/fileStorage", () => ({ thumbnail: thumbnail, }); }), + storeStirlingFile: vi.fn().mockResolvedValue(undefined), getAllFileMetadata: vi.fn().mockResolvedValue([]), cleanup: vi.fn().mockResolvedValue(undefined), }, @@ -169,6 +172,10 @@ describe("Convert Tool - Smart Detection Integration Tests", () => { responseType: "blob", }, ); + + // The output file must be persisted via fileStorage.storeStirlingFile + // so downstream tools see it in the registry. + expect(fileStorage.storeStirlingFile).toHaveBeenCalled(); }); test("should handle unknown file type with file-to-pdf fallback", async () => { @@ -496,6 +503,12 @@ describe("Convert Tool - Smart Detection Integration Tests", () => { }); test("should send correct PDF/A parameters for pdf-to-pdfa conversion", async () => { + // Test files aren't registered in the FileContext stub registry, so + // production warns about the stub/output mismatch - incidental to what + // this test asserts. + allowConsole.warn( + /\[useToolOperation\] Mismatch successInputStubs vs outputs/, + ); const { result: paramsResult } = renderHook( () => useConvertParameters(), { @@ -638,6 +651,9 @@ describe("Convert Tool - Smart Detection Integration Tests", () => { describe("Error Scenarios in Smart Detection", () => { test("should handle partial failures in multi-file processing", async () => { + // Production warns when an individual file in a batch fails; the test + // deliberately drives one mock rejection to exercise that path. + expectConsole.warn(/Failed to convert file doc2\.xyz/); const { result: paramsResult } = renderHook( () => useConvertParameters(), { diff --git a/frontend/editor/src/core/tests/failOnConsole.test.ts b/frontend/editor/src/core/tests/failOnConsole.test.ts new file mode 100644 index 000000000..944917a14 --- /dev/null +++ b/frontend/editor/src/core/tests/failOnConsole.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "vitest"; +import { + allowConsole, + expectConsole, + formatArgs, + matches, +} from "@app/tests/failOnConsole"; + +describe("failOnConsole", () => { + describe("matches", () => { + it("treats a string matcher as a substring test", () => { + expect(matches("foo", "the foo bar")).toBe(true); + expect(matches("foo", "no match here")).toBe(false); + }); + + it("treats a RegExp matcher as a .test() call", () => { + expect(matches(/^\[Tag\]/, "[Tag] something")).toBe(true); + expect(matches(/^\[Tag\]/, "prefix [Tag] something")).toBe(false); + }); + + it("string match is case-sensitive", () => { + expect(matches("Foo", "foo")).toBe(false); + }); + }); + + describe("formatArgs", () => { + it("joins string args with spaces", () => { + expect(formatArgs(["one", "two", "three"])).toBe("one two three"); + }); + + it("stringifies plain objects with JSON", () => { + expect(formatArgs([{ a: 1, b: "x" }])).toBe('{"a":1,"b":"x"}'); + }); + + it("uses the stack for Error instances when present", () => { + const err = new Error("boom"); + const result = formatArgs(["context:", err]); + expect(result).toContain("context:"); + expect(result).toContain("boom"); + }); + + it("falls back to String() when JSON.stringify throws (circular ref)", () => { + const circular: Record = { name: "loop" }; + circular.self = circular; + const out = formatArgs([circular]); + expect(out).toBe("[object Object]"); + }); + + it("handles non-string, non-Error primitives", () => { + expect(formatArgs([42, true, null])).toBe("42 true null"); + }); + }); + + describe("expectConsole / allowConsole integration", () => { + it("expectConsole.error absorbs a matching call and satisfies the expectation", () => { + expectConsole.error(/expected sentinel/); + console.error("an expected sentinel was emitted"); + // No assertion failure here means: the call was absorbed AND + // afterEach won't throw because the expectation matched. + }); + + it("allowConsole.warn absorbs a matching call without asserting it fires", () => { + allowConsole.warn(/optional sentinel/); + // Intentionally do NOT call console.warn. The test should still + // pass because allowConsole is non-required. + }); + + it("allowConsole.warn absorbs the call when it does fire too", () => { + allowConsole.warn(/optional sentinel that does fire/); + console.warn("optional sentinel that does fire now"); + }); + + it("supports a string matcher (substring)", () => { + expectConsole.warn("substring sentinel inside"); + console.warn("a substring sentinel inside a longer message"); + }); + }); +}); diff --git a/frontend/editor/src/core/tests/failOnConsole.ts b/frontend/editor/src/core/tests/failOnConsole.ts new file mode 100644 index 000000000..1c5dc329e --- /dev/null +++ b/frontend/editor/src/core/tests/failOnConsole.ts @@ -0,0 +1,152 @@ +import { afterEach, beforeEach, vi, type MockInstance } from "vitest"; + +type ConsoleMethod = "error" | "warn"; +type Matcher = string | RegExp; + +const WATCHED: ConsoleMethod[] = ["error", "warn"]; + +type Capture = { method: ConsoleMethod; args: unknown[] }; + +type Expectation = { + method: ConsoleMethod; + matcher: Matcher; + /** + * `true` for `expectConsole` (a contract: the test fails if no matching + * call fires); `false` for `allowConsole` (incidental noise: absorbed if + * it happens, ignored if it doesn't). + */ + required: boolean; + matched: boolean; +}; + +const captured: Capture[] = []; +const expectations: Expectation[] = []; + +/** + * Vitest setup that fails any test whose code calls console.error or + * console.warn. The goal is to keep the browser console clean during normal + * usage of the app: React key warnings, act() warnings, deprecation + * notices, and runtime errors should all surface as test failures rather + * than silently scrolling past in CI. + * + * If a test legitimately needs to drive a log: + * - Use `expectConsole.error(/pattern/)` when the log is part of the + * contract under test. The matching call is absorbed AND the test + * fails if it never fires, so the log can't silently disappear later. + * - Use `allowConsole.warn(/pattern/)` when the log is incidental + * (third-party noise, an artefact of a fake JWT, etc.). The matching + * call is absorbed; nothing is asserted. + * + * Anything that doesn't match an expectation/allowance still fails the + * test as before. + */ +export function installFailOnConsole(): void { + const spies: MockInstance[] = []; + + beforeEach(() => { + captured.length = 0; + expectations.length = 0; + spies.length = 0; + for (const method of WATCHED) { + const original = console[method].bind(console); + const spy = vi + .spyOn(console, method) + .mockImplementation((...args: unknown[]) => { + const text = formatArgs(args); + const match = expectations.find( + (e) => e.method === method && matches(e.matcher, text), + ); + if (match) { + match.matched = true; + return; + } + captured.push({ method, args }); + original(...args); + }); + spies.push(spy); + } + }); + + afterEach(() => { + for (const spy of spies.splice(0)) { + spy.mockRestore(); + } + + const unmet = expectations.filter((e) => e.required && !e.matched); + const localCaptured = captured.splice(0); + expectations.length = 0; + + if (unmet.length > 0) { + const lines = unmet + .map((e) => ` console.${e.method}: ${formatMatcher(e.matcher)}`) + .join("\n"); + throw new Error( + `Expected console output never fired:\n${lines}\n\n` + + "If production stopped logging this, either restore the log or " + + "update/remove the expectConsole call.", + ); + } + if (localCaptured.length === 0) return; + const lines = localCaptured + .map((c) => ` console.${c.method}: ${formatArgs(c.args)}`) + .join("\n"); + throw new Error( + `Test produced unexpected console output:\n${lines}\n\n` + + "If the log is expected, capture it with expectConsole.error/warn " + + "(contract) or allowConsole.error/warn (incidental) from " + + "@app/tests/failOnConsole.", + ); + }); +} + +/** + * Register a required console expectation. The test passes only if at + * least one matching call fires; matching calls are absorbed instead of + * failing the test. + */ +export const expectConsole = { + error: (matcher: Matcher) => register("error", matcher, true), + warn: (matcher: Matcher) => register("warn", matcher, true), +}; + +/** + * Register an optional console allowance. Matching calls are absorbed; + * the test does NOT fail if no matching call fires. Use for incidental + * noise (fake-JWT decode warnings, third-party library logs in dev, etc.). + */ +export const allowConsole = { + error: (matcher: Matcher) => register("error", matcher, false), + warn: (matcher: Matcher) => register("warn", matcher, false), +}; + +function register(method: ConsoleMethod, matcher: Matcher, required: boolean) { + expectations.push({ method, matcher, required, matched: false }); +} + +/** Exported for the failOnConsole unit tests; not part of the public API. */ +export function matches(matcher: Matcher, text: string): boolean { + return typeof matcher === "string" + ? text.includes(matcher) + : matcher.test(text); +} + +function formatMatcher(matcher: Matcher): string { + return matcher instanceof RegExp + ? matcher.toString() + : JSON.stringify(matcher); +} + +/** Exported for the failOnConsole unit tests; not part of the public API. */ +export function formatArgs(args: unknown[]): string { + return args.map(formatArg).join(" "); +} + +function formatArg(value: unknown): string { + if (value instanceof Error) return value.stack ?? value.message; + if (typeof value === "string") return value; + try { + return JSON.stringify(value); + } catch { + return String(value); + } +} diff --git a/frontend/editor/src/core/tests/stubbed/console-clean.spec.ts b/frontend/editor/src/core/tests/stubbed/console-clean.spec.ts new file mode 100644 index 000000000..5379f9870 --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/console-clean.spec.ts @@ -0,0 +1,114 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import { errors, type ConsoleMessage, type Page } from "@playwright/test"; + +/** + * Smoke test: standard usage of the app must not produce any + * console.error, console.warn, or uncaught page errors. + * + * Each test disables the fixture's auto-goto (via `test.use({ autoGoto: + * false })`), attaches listeners inline with `attachListeners(page)` BEFORE + * navigating, walks a representative route, lets it settle, then asserts + * the captured buffer is empty. + * + * If you have a legitimate reason a warning fires on a given route + * (third-party library noise we cannot influence, etc.), filter it via the + * `IGNORED` allowlist below, but the default expectation is that the + * console stays clean. Do not add entries casually; prefer fixing the + * underlying issue. + */ + +type ConsoleEntry = { + type: "error" | "warn" | "pageerror"; + text: string; + location?: string; +}; + +const IGNORED: RegExp[] = [ + // Add entries here only with a comment explaining why the warning is + // unavoidable. Default: keep this list empty. +]; + +function shouldIgnore(text: string): boolean { + return IGNORED.some((re) => re.test(text)); +} + +function attachListeners(page: Page): ConsoleEntry[] { + const entries: ConsoleEntry[] = []; + page.on("console", (msg: ConsoleMessage) => { + const type = msg.type(); + if (type !== "error" && type !== "warning") return; + const text = msg.text(); + if (shouldIgnore(text)) return; + const loc = msg.location(); + entries.push({ + type: type === "warning" ? "warn" : "error", + text, + location: loc.url + ? `${loc.url}:${loc.lineNumber}:${loc.columnNumber}` + : undefined, + }); + }); + page.on("pageerror", (err) => { + if (shouldIgnore(err.message)) return; + entries.push({ type: "pageerror", text: err.stack ?? err.message }); + }); + return entries; +} + +function formatEntries(entries: ConsoleEntry[]): string { + return entries + .map( + (e) => + ` [${e.type}] ${e.text}${e.location ? `\n at ${e.location}` : ""}`, + ) + .join("\n"); +} + +async function expectCleanConsole(entries: ConsoleEntry[]) { + expect( + entries, + `Page produced unexpected console output:\n${formatEntries(entries)}`, + ).toEqual([]); +} + +// ─── Routes to sweep ──────────────────────────────────────────────────────── +// +// One entry per route we want to guarantee is console-clean on load. Mirrors +// the most common user entry points; expand cautiously - every entry adds CI +// time and triage surface for new warnings. + +const ROUTES: { name: string; path: string }[] = [ + { name: "landing", path: "/" }, + { name: "files", path: "/files" }, + { name: "compress", path: "/compress" }, + { name: "split", path: "/split" }, + { name: "merge", path: "/merge" }, + { name: "convert", path: "/convert" }, + { name: "rotate", path: "/rotate" }, + { name: "addPageNumbers", path: "/add-page-numbers" }, +]; + +// Disable the fixture's auto-goto so we can attach listeners before any +// navigation happens. Otherwise listeners miss early load-time noise. +test.use({ autoGoto: false }); + +test.describe("Console hygiene: representative routes load cleanly", () => { + for (const route of ROUTES) { + test(`${route.name} (${route.path})`, async ({ page }) => { + const entries = attachListeners(page); + await page.goto(route.path, { waitUntil: "domcontentloaded" }); + // Give async effects (i18n load, lazy chunks, posthog init) a beat to + // surface anything they were going to log. + await page + .waitForLoadState("networkidle", { timeout: 10_000 }) + .catch((err) => { + // networkidle can flake on third-party CDNs; treat ONLY the + // timeout as benign and still run the console assertion on what + // we captured. Anything else (frame detached, navigation abort, + // etc.) is a real problem and should fail the test. + if (!(err instanceof errors.TimeoutError)) throw err; + }); + await expectCleanConsole(entries); + }); + } +}); diff --git a/frontend/editor/src/core/utils/automationConverter.test.ts b/frontend/editor/src/core/utils/automationConverter.test.ts index c03df11b9..88e313c60 100644 --- a/frontend/editor/src/core/utils/automationConverter.test.ts +++ b/frontend/editor/src/core/utils/automationConverter.test.ts @@ -3,6 +3,7 @@ */ import { describe, test, expect } from "vitest"; +import { expectConsole } from "@app/tests/failOnConsole"; import { convertToAutomationConfig, convertToFolderScanningConfig, @@ -111,6 +112,7 @@ describe("automationConverter", () => { }); test("falls back to operation key when no endpoint is registered", () => { + expectConsole.warn(/No endpoint found for operation "unknownTool"/); const automation: AutomationConfig = { ...sampleAutomation, operations: [{ operation: "unknownTool", parameters: {} }], diff --git a/frontend/editor/src/core/utils/scarfTracking.ts b/frontend/editor/src/core/utils/scarfTracking.ts index 4c8d8b6c3..ca4769f35 100644 --- a/frontend/editor/src/core/utils/scarfTracking.ts +++ b/frontend/editor/src/core/utils/scarfTracking.ts @@ -6,8 +6,9 @@ * injected via setScarfConfig() which should be called from a React hook * during app initialization. * - * IMPORTANT: setScarfConfig() must be called before firePixel() will work. - * The initialization hook (useScarfTracking) is mounted in App.tsx. + * firePixel() can be called BEFORE setScarfConfig() runs: pre-config calls + * are queued and replayed once setScarfConfig fires, so the initial-page-load + * pixel from useUrlSync doesn't race the useScarfTracking init effect. * * For testing: Use resetScarfConfig() to clear module state between tests. */ @@ -19,6 +20,11 @@ let isServiceAccepted: ((service: string, category: string) => boolean) | null = null; let lastFiredPathname: string | null = null; let lastFiredTime = 0; +// Pathnames passed to firePixel() before setScarfConfig() has run. Drained +// once configured. Bounded to the most recent few entries since intermediate +// path changes during a slow init are uninteresting. +const pendingPaths: string[] = []; +const PENDING_PATHS_CAP = 8; /** * Configure scarf tracking with app config and consent checker @@ -34,6 +40,10 @@ export function setScarfConfig( configured = true; enableScarf = scarfEnabled; isServiceAccepted = consentChecker; + // Drain anything queued before we were configured. Splice first so a + // re-entrant firePixel from inside the drain can't re-queue. + const queued = pendingPaths.splice(0); + for (const path of queued) firePixel(path); } /** @@ -47,12 +57,10 @@ export function setScarfConfig( * @param pathname - The pathname to track (usually window.location.pathname) */ export function firePixel(pathname: string): void { - // Dev-mode warning if called before initialization + // Pre-init: queue and bail. setScarfConfig() drains. if (!configured) { - console.warn( - "[scarfTracking] firePixel() called before setScarfConfig(). " + - "Ensure useScarfTracking() hook is mounted in App.tsx.", - ); + if (pendingPaths.length >= PENDING_PATHS_CAP) pendingPaths.shift(); + pendingPaths.push(pathname); return; } @@ -91,8 +99,10 @@ export function firePixel(pathname: string): void { * Useful for testing to ensure clean state between test runs */ export function resetScarfConfig(): void { + configured = false; enableScarf = null; isServiceAccepted = null; lastFiredPathname = null; lastFiredTime = 0; + pendingPaths.length = 0; } diff --git a/frontend/editor/src/core/utils/toolSynonyms.ts b/frontend/editor/src/core/utils/toolSynonyms.ts index fe5cdfb00..fbfd2afca 100644 --- a/frontend/editor/src/core/utils/toolSynonyms.ts +++ b/frontend/editor/src/core/utils/toolSynonyms.ts @@ -4,34 +4,16 @@ import { TFunction } from "i18next"; export const getSynonyms = (t: TFunction, toolId: string): string[] => { try { const candidateKeys = [`home.${toolId}.tags`, `${toolId}.tags`]; - const match = candidateKeys + const tags = candidateKeys .map((key) => ({ key, value: t(key) as unknown as string })) - .find(({ key, value }) => value && value !== key); - const tags = match?.value; - const usedKey = match?.key; + .find(({ key, value }) => value && value !== key)?.value; - if (!tags) { - console.warn(`[Tags] Missing tags for tool: ${toolId}`); - return []; - } + if (!tags) return []; - // Split by comma and clean up the tags - const cleanedTags = tags + return tags .split(",") .map((tag: string) => tag.trim()) .filter((tag: string) => tag.length > 0); - - // Log the tags found for this tool - if (cleanedTags.length > 0) { - console.info( - `[Tags] Tool "${toolId}" (${usedKey}) has ${cleanedTags.length} tags:`, - cleanedTags, - ); - } else { - console.warn(`[Tags] Tool "${toolId}" has empty tags value`); - } - - return cleanedTags; } catch (error) { console.error( `[Tags] Failed to get translated synonyms for tool ${toolId}:`, diff --git a/frontend/editor/src/desktop/hooks/useDesktopUpdatePopup.test.ts b/frontend/editor/src/desktop/hooks/useDesktopUpdatePopup.test.ts index 83ef34fd4..d26a20a98 100644 --- a/frontend/editor/src/desktop/hooks/useDesktopUpdatePopup.test.ts +++ b/frontend/editor/src/desktop/hooks/useDesktopUpdatePopup.test.ts @@ -1,5 +1,6 @@ -import { renderHook } from "@testing-library/react"; +import { act, renderHook } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { expectConsole } from "@app/tests/failOnConsole"; // ── Mocks (hoisted by vi.mock) ────────────────────────────────────────────── const invokeMock = vi.fn(); @@ -46,13 +47,21 @@ async function flushMicrotasks() { const AUTO_FAILURE_KEY = "stirling-pdf-updater:autoFailedAt"; -/** Run the hook through its startup timer + async chain. */ +/** + * Run the hook through its startup timer + async chain. The post-render + * timer-driven async work is wrapped in act() so the state updates it + * triggers don't surface as React "not wrapped in act" warnings. + * renderHook already wraps the initial render in act internally, so it + * stays outside. + */ async function runStartup() { renderHook(() => useDesktopUpdatePopup()); - await vi.advanceTimersByTimeAsync(16_000); - await flushMicrotasks(); - await vi.advanceTimersByTimeAsync(0); - await flushMicrotasks(); + await act(async () => { + await vi.advanceTimersByTimeAsync(16_000); + await flushMicrotasks(); + await vi.advanceTimersByTimeAsync(0); + await flushMicrotasks(); + }); } describe("useDesktopUpdatePopup — auto mode", () => { @@ -83,6 +92,7 @@ describe("useDesktopUpdatePopup — auto mode", () => { }); it("does NOT call restart_app when download_and_install_update fails", async () => { + expectConsole.error(/\[useDesktopInstall\] Install failed/); invokeMock.mockImplementation((cmd: string) => { if (cmd === "check_for_update") { return Promise.resolve({ @@ -132,6 +142,9 @@ describe("useDesktopUpdatePopup — auto mode", () => { }); it("skips the install entirely when a recent failure is within the backoff window", async () => { + expectConsole.warn( + /\[DesktopUpdatePopup\] auto-update skipped: recent failure within backoff window/, + ); // Recorded 1 hour ago — well inside the 6-hour backoff. const oneHourAgo = Date.now() - 60 * 60 * 1000; window.localStorage.setItem(AUTO_FAILURE_KEY, String(oneHourAgo)); diff --git a/frontend/editor/src/desktop/services/apiClientSetup.test.ts b/frontend/editor/src/desktop/services/apiClientSetup.test.ts index 140657018..fd46abd72 100644 --- a/frontend/editor/src/desktop/services/apiClientSetup.test.ts +++ b/frontend/editor/src/desktop/services/apiClientSetup.test.ts @@ -1,6 +1,7 @@ import type { AxiosInstance } from "axios"; import { describe, expect, test, vi, beforeEach, afterEach } from "vitest"; import { OPEN_SIGN_IN_EVENT } from "@app/constants/signInEvents"; +import { expectConsole } from "@app/tests/failOnConsole"; // Exercise the error-interceptor logic against a hand-rolled axios-like // client; transitive imports are mocked out so this is a pure unit test. @@ -120,6 +121,7 @@ describe("desktop apiClientSetup - 401 silent-path", () => { }); test("POST 401 without Authorization dispatches sign-in modal event", async () => { + expectConsole.warn(/\[apiClientSetup\] 401 on path/); const { client, handlers } = makeMockClient(); setupApiInterceptors(client as unknown as AxiosInstance); await triggerErrorInterceptor(handlers, { @@ -135,6 +137,7 @@ describe("desktop apiClientSetup - 401 silent-path", () => { }); test("GET 401 without Authorization stays silent (background probe)", async () => { + expectConsole.warn(/\[apiClientSetup\] 401 on path/); const { client, handlers } = makeMockClient(); setupApiInterceptors(client as unknown as AxiosInstance); await triggerErrorInterceptor(handlers, { @@ -159,6 +162,7 @@ describe("desktop apiClientSetup - 401 silent-path", () => { }); test("401 with skipAuthRedirect set never dispatches the modal", async () => { + expectConsole.warn(/\[apiClientSetup\] 401 on path/); const { client, handlers } = makeMockClient(); setupApiInterceptors(client as unknown as AxiosInstance); await triggerErrorInterceptor(handlers, { diff --git a/frontend/editor/src/proprietary/auth/springAuthClient.test.ts b/frontend/editor/src/proprietary/auth/springAuthClient.test.ts index 1258a4d99..d64ac4947 100644 --- a/frontend/editor/src/proprietary/auth/springAuthClient.test.ts +++ b/frontend/editor/src/proprietary/auth/springAuthClient.test.ts @@ -8,6 +8,7 @@ import { } from "@app/auth/springAuthClient"; import { startOAuthNavigation } from "@app/extensions/oauthNavigation"; import apiClient from "@app/services/apiClient"; +import { allowConsole, expectConsole } from "@app/tests/failOnConsole"; import { AxiosError, type AxiosResponse, @@ -89,9 +90,18 @@ describe("SpringAuthClient", () => { ); vi.mocked(apiClient.get).mockRejectedValueOnce(mockError); + vi.mocked(apiClient.post).mockRejectedValueOnce(mockError); const result = await springAuth.getSession(); + // A 401 from /me triggers an explicit refresh attempt before treating + // the session as invalid; lock that recovery contract in so future + // refactors can't quietly skip it. + expect(apiClient.post).toHaveBeenCalledWith( + "/api/v1/auth/refresh", + null, + expect.any(Object), + ); expect(localStorage.getItem("stirling_jwt")).toBeNull(); expect(result.data.session).toBeNull(); // 401 is handled gracefully, so error should be null @@ -117,9 +127,18 @@ describe("SpringAuthClient", () => { ); vi.mocked(apiClient.get).mockRejectedValueOnce(mockError); + vi.mocked(apiClient.post).mockRejectedValueOnce(mockError); const result = await springAuth.getSession(); + // A 403 from /me triggers an explicit refresh attempt before treating + // the session as invalid; lock that recovery contract in so future + // refactors can't quietly skip it. + expect(apiClient.post).toHaveBeenCalledWith( + "/api/v1/auth/refresh", + null, + expect.any(Object), + ); expect(localStorage.getItem("stirling_jwt")).toBeNull(); expect(result.data.session).toBeNull(); // 403 is handled gracefully, so error should be null @@ -129,6 +148,9 @@ describe("SpringAuthClient", () => { describe("signInWithPassword", () => { it("should successfully sign in with email and password", async () => { + // The fake token isn't a real JWT, so calculateAdaptiveIntervals warns + // about defaults - incidental to what this test verifies. + allowConsole.warn(/Cannot decode token for adaptive intervals/); const credentials = { email: "test@example.com", password: "password123", @@ -176,6 +198,7 @@ describe("SpringAuthClient", () => { }); it("should return error on failed login", async () => { + expectConsole.error(/\[SpringAuth\] signInWithPassword error/); const credentials = { email: "wrong@example.com", password: "wrongpassword", @@ -236,6 +259,7 @@ describe("SpringAuthClient", () => { }); it("should return error on failed registration", async () => { + expectConsole.error(/\[SpringAuth\] signUp error/); const credentials = { email: "existing@example.com", password: "password123", @@ -283,6 +307,7 @@ describe("SpringAuthClient", () => { }); it("should clear JWT even if logout request fails", async () => { + expectConsole.error(/\[SpringAuth\] signOut error/); const mockToken = "jwt-to-clear"; localStorage.setItem("stirling_jwt", mockToken); @@ -301,6 +326,9 @@ describe("SpringAuthClient", () => { describe("refreshSession", () => { it("should refresh JWT token successfully", async () => { + // The fake token isn't a real JWT, so calculateAdaptiveIntervals warns + // about defaults - incidental to what this test verifies. + allowConsole.warn(/Cannot decode token for adaptive intervals/); const newToken = "refreshed-jwt-token"; const mockUser = { id: "123", diff --git a/frontend/editor/src/proprietary/auth/springAuthClient.ts b/frontend/editor/src/proprietary/auth/springAuthClient.ts index 91cef2e5f..5d75605e3 100644 --- a/frontend/editor/src/proprietary/auth/springAuthClient.ts +++ b/frontend/editor/src/proprietary/auth/springAuthClient.ts @@ -397,22 +397,19 @@ class SpringAuthClient { // console.debug('[SpringAuth] getSession: Session retrieved successfully'); return { data: { session }, error: null }; } catch (error: unknown) { - console.error("[SpringAuth] getSession error:", error); - - // If 401/403, token is invalid - try explicit refresh + // 401/403 during getSession is the normal "token expired or invalid" + // path - handled via refresh + JWT clear. const status = getHttpStatus(error); if (status === 401 || status === 403) { - // A 401 during startup can be a race with a concurrent refresh. Try one - // explicit refresh before treating the session as invalid. const refreshResult = await this.refreshSession(); if (!refreshResult.error && refreshResult.data.session) { return refreshResult; } localStorage.removeItem("stirling_jwt"); - console.debug("[SpringAuth] getSession: Not authenticated"); return { data: { session: null }, error: null }; } + console.error("[SpringAuth] getSession error:", error); // Don't clear token for other errors (e.g., backend not ready, network issues) // The token is still valid, just can't verify it right now return { @@ -739,10 +736,11 @@ class SpringAuthClient { return { data: { session }, error: null }; } catch (error: unknown) { - console.error("[SpringAuth] refreshSession error:", error); localStorage.removeItem("stirling_jwt"); - // Handle different error statuses + // 401/403 means the refresh token is no longer valid - normal expired + // state, not an error worth surfacing. Other statuses (network, backend + // down) ARE worth logging. const status = getHttpStatus(error); if (status === 401 || status === 403) { return { @@ -751,6 +749,7 @@ class SpringAuthClient { }; } + console.error("[SpringAuth] refreshSession error:", error); return { data: { session: null }, error: { message: getErrorMessage(error, "Token refresh failed") }, diff --git a/frontend/editor/src/proprietary/components/shared/stripeCheckout/stages/PaymentStage.tsx b/frontend/editor/src/proprietary/components/shared/stripeCheckout/stages/PaymentStage.tsx index a84db823a..1e9d7acec 100644 --- a/frontend/editor/src/proprietary/components/shared/stripeCheckout/stages/PaymentStage.tsx +++ b/frontend/editor/src/proprietary/components/shared/stripeCheckout/stages/PaymentStage.tsx @@ -1,4 +1,4 @@ -import React from "react"; +import React, { useMemo } from "react"; import { Stack, Text, Loader } from "@mantine/core"; import { useTranslation } from "react-i18next"; import { loadStripe } from "@stripe/stripe-js"; @@ -8,9 +8,7 @@ import { } from "@stripe/react-stripe-js"; import { PlanTier } from "@app/services/licenseService"; -// Load Stripe once const STRIPE_KEY = import.meta.env.VITE_STRIPE_PUBLISHABLE_KEY; -const stripePromise = STRIPE_KEY ? loadStripe(STRIPE_KEY) : null; interface PaymentStageProps { clientSecret: string | null; @@ -24,6 +22,14 @@ export const PaymentStage: React.FC = ({ onPaymentComplete, }) => { const { t } = useTranslation(); + // Load Stripe.js lazily, only when PaymentStage mounts. Loading at module + // scope pulled the Stripe script into every page (CheckoutContext is in + // AppProviders), which triggered the dev "HTTPS required" warning on every + // non-payment route. + const stripePromise = useMemo( + () => (STRIPE_KEY ? loadStripe(STRIPE_KEY) : null), + [], + ); // Show loading while creating checkout session if (!clientSecret || !selectedPlan) { diff --git a/frontend/editor/src/proprietary/routes/AuthCallback.test.tsx b/frontend/editor/src/proprietary/routes/AuthCallback.test.tsx index fc05574fb..449135838 100644 --- a/frontend/editor/src/proprietary/routes/AuthCallback.test.tsx +++ b/frontend/editor/src/proprietary/routes/AuthCallback.test.tsx @@ -6,6 +6,7 @@ import { POST_LOGIN_REDIRECT_STORAGE_KEY, springAuth, } from "@app/auth/springAuthClient"; +import { expectConsole } from "@app/tests/failOnConsole"; // Mock springAuth; keep the real redirect-path helpers. vi.mock("@app/auth/springAuthClient", async () => { @@ -90,6 +91,7 @@ describe("AuthCallback", () => { }); it("should redirect to login when no access token in hash", async () => { + expectConsole.error(/\[AuthCallback\] No access_token in URL fragment/); // No hash or empty hash window.location.hash = ""; @@ -109,6 +111,7 @@ describe("AuthCallback", () => { }); it("should redirect to login when token validation fails", async () => { + expectConsole.error(/\[AuthCallback\] Failed to validate token/); const invalidToken = "invalid-oauth-token"; window.location.hash = `#access_token=${invalidToken}`; @@ -137,6 +140,7 @@ describe("AuthCallback", () => { }); it("should handle errors gracefully", async () => { + expectConsole.error(/\[AuthCallback\] Authentication failed/); const mockToken = "error-token"; window.location.hash = `#access_token=${mockToken}`; diff --git a/frontend/editor/src/proprietary/routes/AuthCallback.tsx b/frontend/editor/src/proprietary/routes/AuthCallback.tsx index 32f81b841..fd2754ee8 100644 --- a/frontend/editor/src/proprietary/routes/AuthCallback.tsx +++ b/frontend/editor/src/proprietary/routes/AuthCallback.tsx @@ -18,42 +18,16 @@ export default function AuthCallback() { const navigate = useNavigate(); const processingRef = useRef(false); - // Log component lifecycle useEffect(() => { - const mountId = Math.random().toString(36).substring(7); - console.log(`[AuthCallback:${mountId}] 🔵 Component mounted`); - return () => { - console.log(`[AuthCallback:${mountId}] 🔴 Component unmounting`); - }; - }, []); + const startedAt = performance.now(); + const elapsed = () => `${(performance.now() - startedAt).toFixed(0)}ms`; - useEffect(() => { const handleCallback = async () => { - const startTime = performance.now(); - const executionId = Math.random().toString(36).substring(7); - - console.log( - `[AuthCallback:${executionId}] ════════════════════════════════════`, - ); - console.log( - `[AuthCallback:${executionId}] Starting authentication callback`, - ); - console.log(`[AuthCallback:${executionId}] URL: ${window.location.href}`); - console.log( - `[AuthCallback:${executionId}] Hash: ${window.location.hash}`, - ); - console.log( - `[AuthCallback:${executionId}] Document readyState: ${document.readyState}`, - ); - if ( typeof window !== "undefined" && window.sessionStorage.getItem("stirling_sso_auto_login_logged_out") === "1" ) { - console.warn( - `[AuthCallback:${executionId}] ⚠️ Logout block active, skipping token processing`, - ); navigate("/login", { replace: true, state: { error: "You have been signed out. Please sign in again." }, @@ -62,30 +36,16 @@ export default function AuthCallback() { } // Prevent double execution (React 18 Strict Mode + navigate dependency) - if (processingRef.current) { - console.warn( - `[AuthCallback:${executionId}] ⚠️ Already processing, skipping duplicate execution`, - ); - console.warn( - `[AuthCallback:${executionId}] This is expected in React Strict Mode (development)`, - ); - return; - } + if (processingRef.current) return; processingRef.current = true; try { - console.log( - `[AuthCallback:${executionId}] Step 1: Extracting token from URL fragment`, - ); - - // Extract JWT from URL fragment (#access_token=...) - const hash = window.location.hash.substring(1); // Remove '#' - const params = new URLSearchParams(hash); - const token = params.get("access_token"); + const hash = window.location.hash.substring(1); + const token = new URLSearchParams(hash).get("access_token"); if (!token) { console.error( - `[AuthCallback:${executionId}] ❌ No access_token in URL fragment`, + `[AuthCallback] No access_token in URL fragment (${elapsed()})`, ); navigate("/login", { replace: true, @@ -94,39 +54,13 @@ export default function AuthCallback() { return; } - console.log( - `[AuthCallback:${executionId}] ✓ Token extracted (length: ${token.length})`, - ); - console.log( - `[AuthCallback:${executionId}] Step 2: Storing JWT in localStorage`, - ); - - // Store JWT in localStorage localStorage.setItem("stirling_jwt", token); - console.log( - `[AuthCallback:${executionId}] ✓ JWT stored in localStorage`, - ); - - console.log( - `[AuthCallback:${executionId}] Step 3: Dispatching 'jwt-available' event`, - ); - // Dispatch custom event for other components to react to JWT availability window.dispatchEvent(new CustomEvent("jwt-available")); - console.log(`[AuthCallback:${executionId}] ✓ Event dispatched`); - console.log( - `[AuthCallback:${executionId}] Elapsed after jwt-available: ${(performance.now() - startTime).toFixed(2)}ms`, - ); - console.log( - `[AuthCallback:${executionId}] Step 4: Validating token with backend`, - ); - // Validate the token and load user info - // This calls /api/v1/auth/me with the JWT to get user details const { data, error } = await springAuth.getSession(); - if (error || !data.session) { console.error( - `[AuthCallback:${executionId}] ❌ Failed to validate token:`, + `[AuthCallback] Failed to validate token (${elapsed()}):`, error, ); localStorage.removeItem("stirling_jwt"); @@ -137,68 +71,21 @@ export default function AuthCallback() { return; } - console.log( - `[AuthCallback:${executionId}] ✓ Token validated, user: ${data.session.user.username}`, - ); - console.log( - `[AuthCallback:${executionId}] Step 5: Running platform-specific callback handlers`, - ); - await handleAuthCallbackSuccess(token); - console.log( - `[AuthCallback:${executionId}] ✓ Callback handlers complete`, - ); - console.log( - `[AuthCallback:${executionId}] Step 6: Waiting for context stabilization`, - ); - // Wait for all context providers to process jwt-available event // This prevents infinite render loop when coming from cross-domain SAML redirect await new Promise((resolve) => setTimeout(resolve, 100)); - console.log( - `[AuthCallback:${executionId}] Elapsed after stabilization wait: ${(performance.now() - startTime).toFixed(2)}ms`, - ); const target = consumePostLoginRedirectPath() ?? "/"; - console.log( - `[AuthCallback:${executionId}] Step 7: Navigating to ${target}`, + console.info( + `[AuthCallback] Authenticated ${data.session.user.username} in ${elapsed()}, navigating to ${target}`, ); navigate(target, { replace: true }); - - const duration = performance.now() - startTime; - console.log( - `[AuthCallback:${executionId}] ✓ Authentication complete (${duration.toFixed(2)}ms)`, - ); - console.log( - `[AuthCallback:${executionId}] ════════════════════════════════════`, - ); } catch (error) { - const duration = performance.now() - startTime; console.error( - `[AuthCallback:${executionId}] ════════════════════════════════════`, - ); - console.error( - `[AuthCallback:${executionId}] ❌ FATAL ERROR during authentication`, - ); - console.error(`[AuthCallback:${executionId}] Error:`, error); - console.error( - `[AuthCallback:${executionId}] Error name:`, - (error as Error)?.name, - ); - console.error( - `[AuthCallback:${executionId}] Error message:`, - (error as Error)?.message, - ); - console.error( - `[AuthCallback:${executionId}] Error stack:`, - (error as Error)?.stack, - ); - console.error( - `[AuthCallback:${executionId}] Duration before failure: ${duration.toFixed(2)}ms`, - ); - console.error( - `[AuthCallback:${executionId}] ════════════════════════════════════`, + `[AuthCallback] Authentication failed (${elapsed()}):`, + error, ); navigate("/login", { replace: true, diff --git a/frontend/editor/src/proprietary/routes/Login.test.tsx b/frontend/editor/src/proprietary/routes/Login.test.tsx index 0a4b79324..6b9306a72 100644 --- a/frontend/editor/src/proprietary/routes/Login.test.tsx +++ b/frontend/editor/src/proprietary/routes/Login.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; -import { render, screen, waitFor } from "@testing-library/react"; +import { act, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { BrowserRouter, MemoryRouter } from "react-router-dom"; import { MantineProvider } from "@mantine/core"; @@ -176,7 +176,7 @@ describe("Login", () => { }); }); - it("should show loading state while auth is loading", () => { + it("should show loading state while auth is loading", async () => { vi.mocked(useAuth).mockReturnValue({ session: null, user: null, @@ -187,13 +187,15 @@ describe("Login", () => { refreshSession: vi.fn(), }); - render( - - - - - , - ); + await act(async () => { + render( + + + + + , + ); + }); // Component shouldn't redirect or show form while loading expect(mockNavigate).not.toHaveBeenCalled(); @@ -510,26 +512,30 @@ describe("Login", () => { expect(springAuth.signInWithPassword).not.toHaveBeenCalled(); }); - it("should display session expired message from URL param", () => { - render( - - - - - , - ); + it("should display session expired message from URL param", async () => { + await act(async () => { + render( + + + + + , + ); + }); expect(screen.getByText(/session.*expired/i)).toBeInTheDocument(); }); - it("should display account created success message", () => { - render( - - - - - , - ); + it("should display account created success message", async () => { + await act(async () => { + render( + + + + + , + ); + }); expect(screen.getByText(/account created/i)).toBeInTheDocument(); }); diff --git a/frontend/editor/src/proprietary/routes/Login.tsx b/frontend/editor/src/proprietary/routes/Login.tsx index 22d9c57e1..85c42fce8 100644 --- a/frontend/editor/src/proprietary/routes/Login.tsx +++ b/frontend/editor/src/proprietary/routes/Login.tsx @@ -287,8 +287,6 @@ export default function Login() { setPostLoginRedirectPath(returnPath); } - console.log(`[Login] Signing in with provider: ${provider}`); - // Redirect to Spring OAuth2 endpoint using the actual provider ID from backend // The backend returns the correct registration ID (e.g., 'authentik', 'oidc', 'keycloak') const { error } = await springAuth.signInWithOAuth({ @@ -520,8 +518,6 @@ export default function Login() { setError(null); clearLogoutBlock(); - console.log("[Login] Signing in with email:", email); - const { user, session, error } = await springAuth.signInWithPassword({ email: email.trim(), password: password, @@ -529,13 +525,11 @@ export default function Login() { }); if (error) { - console.error("[Login] Email sign in error:", error); setError(error.message); if (error.mfaRequired || error.code === "invalid_mfa_code") { setRequiresMfa(true); } } else if (user && session) { - console.log("[Login] Email sign in successful"); clearLogoutBlock(); setRequiresMfa(false); setMfaCode(""); diff --git a/frontend/editor/src/saas/components/shared/StripeCheckoutSaas.tsx b/frontend/editor/src/saas/components/shared/StripeCheckoutSaas.tsx index fffefc91d..43c96a9b3 100644 --- a/frontend/editor/src/saas/components/shared/StripeCheckoutSaas.tsx +++ b/frontend/editor/src/saas/components/shared/StripeCheckoutSaas.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from "react"; +import React, { useEffect, useMemo, useState } from "react"; import { Modal, Button, Text, Alert, Loader, Stack } from "@mantine/core"; import { useTranslation } from "react-i18next"; import { loadStripe } from "@stripe/stripe-js"; @@ -9,7 +9,7 @@ import { import { supabase } from "@app/auth/supabase"; import { Z_INDEX_OVER_SETTINGS_MODAL } from "@app/styles/zIndex"; -const stripePromise = loadStripe(import.meta.env.VITE_STRIPE_PUBLISHABLE_KEY); +const STRIPE_KEY = import.meta.env.VITE_STRIPE_PUBLISHABLE_KEY; export type PurchaseType = "subscription" | "credits"; export type CreditsPack = "xsmall" | "small" | "medium" | "large" | null; @@ -68,6 +68,11 @@ const StripeCheckout: React.FC = ({ }) => { const { t } = useTranslation(); const [state, setState] = useState({ status: "idle" }); + // Load Stripe.js lazily, only when this checkout component mounts. Loading + // at module scope pulled the Stripe script into every page that imports + // this file, which triggered the dev "HTTPS required" warning on every + // non-payment route. + const stripePromise = useMemo(() => loadStripe(STRIPE_KEY), []); const createCheckoutSession = async () => { try { diff --git a/frontend/editor/src/saas/services/apiClient.test.ts b/frontend/editor/src/saas/services/apiClient.test.ts index 01a964938..56480bb7c 100644 --- a/frontend/editor/src/saas/services/apiClient.test.ts +++ b/frontend/editor/src/saas/services/apiClient.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import type { Session, AuthError } from "@supabase/supabase-js"; import { supabase } from "@app/auth/supabase"; +import { expectConsole } from "@app/tests/failOnConsole"; // Mock supabase vi.mock("@app/auth/supabase", () => ({ @@ -177,6 +178,7 @@ describe("apiClient", () => { }); it("should handle refresh token failure", async () => { + expectConsole.error(/\[API Client\] Token refresh failed/); const oldToken = "old-token"; const oldSession = { diff --git a/frontend/editor/src/saas/setupTests.ts b/frontend/editor/src/saas/setupTests.ts index c461557ff..2c5f53e27 100644 --- a/frontend/editor/src/saas/setupTests.ts +++ b/frontend/editor/src/saas/setupTests.ts @@ -1,5 +1,8 @@ import "@testing-library/jest-dom"; import { vi } from "vitest"; +import { installFailOnConsole } from "@app/tests/failOnConsole"; + +installFailOnConsole(); // Mock localStorage for tests const localStorageMock = (() => {