From fe1dae8b5f9a2c717630392e43e27f1bb9f9767c Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 30 Oct 2026 23:24:08 +0100 Subject: [PATCH 01/10] fix: restore server error reporting --- instrumentation.ts | 17 +++--- next.config.ts | 16 ------ sentry.edge.config.ts | 22 -------- sentry.server.config.ts | 18 ++---- src/app/api/mobile/trpc/[trpc]/route.ts | 11 ++-- src/app/api/trpc/[trpc]/route.ts | 8 +-- src/instrumentation-client.ts | 16 ++---- src/instrumentation.test.ts | 70 +++++++++++++++++++++++ src/instrumentation.ts | 13 ----- src/sentry-config.test.ts | 12 ++-- src/server/api/trpc-errors.test.ts | 74 +++++++++++++++++++++++++ src/server/api/trpc-errors.ts | 12 ++++ 12 files changed, 187 insertions(+), 102 deletions(-) delete mode 100644 sentry.edge.config.ts create mode 100644 src/instrumentation.test.ts delete mode 100644 src/instrumentation.ts create mode 100644 src/server/api/trpc-errors.test.ts create mode 100644 src/server/api/trpc-errors.ts diff --git a/instrumentation.ts b/instrumentation.ts index 0827d94b9..399995591 100644 --- a/instrumentation.ts +++ b/instrumentation.ts @@ -1,15 +1,14 @@ -/** - * Next.js Instrumentation - * This file runs once when the Next.js server starts - * for one-time initialization of services - */ - +import * as Sentry from '@sentry/nextjs' import { PHASE_PRODUCTION_BUILD } from 'next/constants' export async function register() { - if (process.env.NEXT_RUNTIME !== 'nodejs') return if (process.env.NEXT_PHASE === PHASE_PRODUCTION_BUILD) return - const { initializeServerOnce } = await import('./src/server/init') - await initializeServerOnce() + if (process.env.NEXT_RUNTIME === 'nodejs') { + await import('./sentry.server.config') + const { initializeServerOnce } = await import('./src/server/init') + await initializeServerOnce() + } } + +export const onRequestError = Sentry.captureRequestError diff --git a/next.config.ts b/next.config.ts index 9bee1bcb4..b0e117116 100644 --- a/next.config.ts +++ b/next.config.ts @@ -324,32 +324,16 @@ const withBundleAnalyzer = NextBundleAnalyzer({ const analyzedConfig = withBundleAnalyzer(nextConfig) const sentryBuildOptions = { - // For all available options, see: - // https://docs.sentry.io/platforms/javascript/guides/nextjs/configuration/build/ - org: 'hexelnet', project: 'emuready', - - // Only print logs for uploading source maps in CI silent: !process.env.CI, - - // Upload a larger set of source maps for prettier stack traces (increases build time) widenClientFileUpload: true, - sourcemaps: { disable: !isVercelBuild, }, - - // Uncomment to route browser requests to Sentry through a Next.js rewrite to circumvent ad-blockers. - // This can increase your server load as well as your hosting bill. - // Note: Check that the configured route will not match with your Next.js middleware, otherwise reporting of client- - // side errors will fail. - // tunnelRoute: "/monitoring", - bundleSizeOptimizations: { excludeDebugStatements: true, }, - telemetry: false, } diff --git a/sentry.edge.config.ts b/sentry.edge.config.ts deleted file mode 100644 index f24441da4..000000000 --- a/sentry.edge.config.ts +++ /dev/null @@ -1,22 +0,0 @@ -// This file configures the initialization of Sentry for edge features (middleware, edge routes, and so on). -// The config you add here will be used whenever one of the edge features is loaded. -// Note that this config is unrelated to the Vercel Edge Runtime and is also required when running locally. -// https://docs.sentry.io/platforms/javascript/guides/nextjs/ - -import * as Sentry from '@sentry/nextjs' - -// Only enable Sentry when explicitly configured for this deployment. -if (process.env.NEXT_PUBLIC_ENABLE_SENTRY === 'true') { - Sentry.init({ - dsn: 'https://85ca585e45005d8786e361c3456518bf@o74828.ingest.us.sentry.io/4509717207318529', - - // Define how likely traces are sampled. Adjust this value in production, or use tracesSampler for greater control. - tracesSampleRate: 1, - - // Enable logs to be sent to Sentry - enableLogs: true, - - // Setting this option to true will print useful information to the console while you're setting up Sentry. - debug: false, - }) -} diff --git a/sentry.server.config.ts b/sentry.server.config.ts index 15f1fcbb8..398e66d98 100644 --- a/sentry.server.config.ts +++ b/sentry.server.config.ts @@ -1,21 +1,13 @@ -// This file configures the initialization of Sentry on the server. -// The config you add here will be used whenever the server handles a request. -// https://docs.sentry.io/platforms/javascript/guides/nextjs/ - import * as Sentry from '@sentry/nextjs' -// Only enable Sentry when explicitly configured for this deployment. if (process.env.NEXT_PUBLIC_ENABLE_SENTRY === 'true') { Sentry.init({ dsn: 'https://85ca585e45005d8786e361c3456518bf@o74828.ingest.us.sentry.io/4509717207318529', - - // Define how likely traces are sampled. Adjust this value in production, or use tracesSampler for greater control. - tracesSampleRate: 1, - - // Enable logs to be sent to Sentry + environment: process.env.NEXT_PUBLIC_APP_ENV, + release: process.env.APP_VERSION, + sampleRate: 1, + tracesSampleRate: 0.1, enableLogs: true, - - // Setting this option to true will print useful information to the console while you're setting up Sentry. - debug: false, + sendDefaultPii: false, }) } diff --git a/src/app/api/mobile/trpc/[trpc]/route.ts b/src/app/api/mobile/trpc/[trpc]/route.ts index ecfe2b4ca..57747f587 100644 --- a/src/app/api/mobile/trpc/[trpc]/route.ts +++ b/src/app/api/mobile/trpc/[trpc]/route.ts @@ -1,9 +1,11 @@ import { fetchRequestHandler } from '@trpc/server/adapters/fetch' import { connection, type NextRequest, NextResponse } from 'next/server' import { getCORSHeaders } from '@/lib/cors' +import { logger } from '@/lib/logger' import { createMobileTRPCFetchContext } from '@/server/api/mobileContext' import { mobileRouter } from '@/server/api/routers/mobile' import { getTRPCResponseCacheHeaders, TRPC_PRIVATE_CACHE_CONTROL } from '@/server/api/trpc-cache' +import { reportTRPCError } from '@/server/api/trpc-errors' // Get CORS headers with additional tRPC headers function getTRPCCorsHeaders(request: NextRequest) { @@ -68,12 +70,7 @@ const handler = async (req: NextRequest) => { req: correctedRequest, router: mobileRouter, createContext: createMobileTRPCFetchContext, - onError: - process.env.NODE_ENV === 'development' - ? ({ path, error }: { path?: string; error: Error }) => { - console.error(`❌ Mobile tRPC failed on ${path ?? ''}: ${error.message}`) - } - : undefined, + onError: (options) => reportTRPCError(options.error, options.path), responseMeta(opts) { return { headers: { @@ -102,7 +99,7 @@ const handler = async (req: NextRequest) => { return response } catch (error) { // If anything fails, return an error response with CORS headers - console.error('Mobile tRPC handler error:', error) + logger.error('[Mobile tRPC] Handler error', error) // Server-side error filtering - don't expose details in production const errorMessage = diff --git a/src/app/api/trpc/[trpc]/route.ts b/src/app/api/trpc/[trpc]/route.ts index 0f39cf772..fbf5a81f0 100644 --- a/src/app/api/trpc/[trpc]/route.ts +++ b/src/app/api/trpc/[trpc]/route.ts @@ -3,6 +3,7 @@ import { connection, type NextRequest } from 'next/server' import { appRouter } from '@/server/api/root' import { createAppRouterTRPCContext } from '@/server/api/trpc' import { getTRPCResponseCacheHeaders } from '@/server/api/trpc-cache' +import { reportTRPCError } from '@/server/api/trpc-errors' const handler = async (req: NextRequest) => { return fetchRequestHandler({ @@ -10,12 +11,7 @@ const handler = async (req: NextRequest) => { req, router: appRouter, createContext: createAppRouterTRPCContext, - onError: - process.env.NODE_ENV === 'development' - ? ({ path, error }) => { - console.error(`❌ tRPC failed on ${path ?? ''}: ${error.message}`) - } - : undefined, + onError: (options) => reportTRPCError(options.error, options.path), responseMeta(opts) { return { headers: getTRPCResponseCacheHeaders({ diff --git a/src/instrumentation-client.ts b/src/instrumentation-client.ts index 28394f6c9..aea206f5f 100644 --- a/src/instrumentation-client.ts +++ b/src/instrumentation-client.ts @@ -1,23 +1,17 @@ -// This file configures the initialization of Sentry on the client. -// The added config here will be used whenever a users loads a page in their browser. -// https://docs.sentry.io/platforms/javascript/guides/nextjs/ - import * as Sentry from '@sentry/nextjs' import { env } from '@/lib/env' if (env.ENABLE_SENTRY) { Sentry.init({ dsn: 'https://85ca585e45005d8786e361c3456518bf@o74828.ingest.us.sentry.io/4509717207318529', - + environment: env.APP_ENV, integrations: [Sentry.replayIntegration()], - - tracesSampleRate: 1, + sampleRate: 1, + tracesSampleRate: 0.1, enableLogs: true, - + sendDefaultPii: false, replaysSessionSampleRate: 0.1, - replaysOnErrorSampleRate: 1.0, - - debug: false, + replaysOnErrorSampleRate: 1, beforeSend(event, hint) { const error = hint?.originalException diff --git a/src/instrumentation.test.ts b/src/instrumentation.test.ts new file mode 100644 index 000000000..62cbbbaed --- /dev/null +++ b/src/instrumentation.test.ts @@ -0,0 +1,70 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + init: vi.fn(), + captureRequestError: vi.fn(), + initializeServerOnce: vi.fn(), +})) + +vi.mock('@sentry/nextjs', () => ({ + init: mocks.init, + captureRequestError: mocks.captureRequestError, +})) +vi.mock('./server/init', () => ({ initializeServerOnce: mocks.initializeServerOnce })) + +afterEach(() => { + vi.unstubAllEnvs() + vi.resetModules() + vi.clearAllMocks() +}) + +async function loadInstrumentation(runtime: string, enabled = true) { + vi.resetModules() + vi.stubEnv('NEXT_RUNTIME', runtime) + vi.stubEnv('NEXT_PUBLIC_ENABLE_SENTRY', String(enabled)) + vi.stubEnv('NEXT_PHASE', '') + return import('../instrumentation') +} + +describe('server instrumentation entry point', () => { + it('initializes Sentry before starting Node server services', async () => { + const instrumentation = await loadInstrumentation('nodejs') + await instrumentation.register() + + expect(mocks.init).toHaveBeenCalledTimes(1) + expect(mocks.initializeServerOnce).toHaveBeenCalledTimes(1) + expect(mocks.init.mock.invocationCallOrder[0]).toBeLessThan( + mocks.initializeServerOnce.mock.invocationCallOrder[0] ?? 0, + ) + }) + + it('does not initialize Node services in an unsupported runtime', async () => { + const instrumentation = await loadInstrumentation('edge') + await instrumentation.register() + + expect(mocks.init).not.toHaveBeenCalled() + expect(mocks.initializeServerOnce).not.toHaveBeenCalled() + }) + + it('starts services with Sentry disabled', async () => { + const instrumentation = await loadInstrumentation('nodejs', false) + await instrumentation.register() + + expect(mocks.init).not.toHaveBeenCalled() + expect(mocks.initializeServerOnce).toHaveBeenCalledTimes(1) + }) + + it('skips runtime initialization during the production build', async () => { + const instrumentation = await loadInstrumentation('nodejs') + vi.stubEnv('NEXT_PHASE', 'phase-production-build') + await instrumentation.register() + + expect(mocks.init).not.toHaveBeenCalled() + expect(mocks.initializeServerOnce).not.toHaveBeenCalled() + }) + + it('exports the Sentry server-request error handler', async () => { + const instrumentation = await loadInstrumentation('nodejs') + expect(instrumentation).toHaveProperty('onRequestError', mocks.captureRequestError) + }) +}) diff --git a/src/instrumentation.ts b/src/instrumentation.ts deleted file mode 100644 index af8537305..000000000 --- a/src/instrumentation.ts +++ /dev/null @@ -1,13 +0,0 @@ -import * as Sentry from '@sentry/nextjs' - -export async function register() { - if (process.env.NEXT_RUNTIME === 'nodejs') { - await import('../sentry.server.config') - } - - if (process.env.NEXT_RUNTIME === 'edge') { - await import('../sentry.edge.config') - } -} - -export const onRequestError = Sentry.captureRequestError diff --git a/src/sentry-config.test.ts b/src/sentry-config.test.ts index 9aebfa0f5..3dfac3fe5 100644 --- a/src/sentry-config.test.ts +++ b/src/sentry-config.test.ts @@ -15,7 +15,6 @@ async function loadSentryConfigs(overrides: Record) { } await import('../sentry.server.config') - await import('../sentry.edge.config') } afterEach(() => { @@ -24,7 +23,7 @@ afterEach(() => { vi.clearAllMocks() }) -describe('Sentry server and edge config', () => { +describe('Sentry server config', () => { it('does not initialize when the Sentry flag is disabled', async () => { await loadSentryConfigs({ NODE_ENV: 'production', @@ -35,17 +34,20 @@ describe('Sentry server and edge config', () => { expect(sentry.init).not.toHaveBeenCalled() }) - it('initializes server and edge Sentry when the Sentry flag is enabled', async () => { + it.each(['production', 'staging'])('labels server errors for %s', async (environment) => { await loadSentryConfigs({ NODE_ENV: 'production', - NEXT_PUBLIC_APP_ENV: 'production', + NEXT_PUBLIC_APP_ENV: environment, NEXT_PUBLIC_ENABLE_SENTRY: 'true', + APP_VERSION: 'test-release', }) - expect(sentry.init).toHaveBeenCalledTimes(2) + expect(sentry.init).toHaveBeenCalledTimes(1) expect(sentry.init).toHaveBeenCalledWith( expect.objectContaining({ dsn: expect.stringContaining('ingest.us.sentry.io'), + environment, + release: 'test-release', }), ) }) diff --git a/src/server/api/trpc-errors.test.ts b/src/server/api/trpc-errors.test.ts new file mode 100644 index 000000000..11ff04895 --- /dev/null +++ b/src/server/api/trpc-errors.test.ts @@ -0,0 +1,74 @@ +import { initTRPC, TRPCError } from '@trpc/server' +import { fetchRequestHandler } from '@trpc/server/adapters/fetch' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { z } from 'zod' +import { reportTRPCError } from './trpc-errors' + +const mocks = vi.hoisted(() => ({ error: vi.fn() })) +vi.mock('@/lib/logger', () => ({ logger: { error: mocks.error } })) + +const trpc = initTRPC.create() +const inputSchema = z.object({ receipt: z.string() }) + +async function requestFailure(error: Error) { + const router = trpc.router({ + probe: trpc.procedure.input(inputSchema).query(() => { + throw error + }), + }) + const url = new URL('https://example.test/api/trpc/probe') + url.searchParams.set('input', JSON.stringify({ receipt: 'private-test-receipt' })) + + return fetchRequestHandler({ + endpoint: '/api/trpc', + req: new Request(url), + router, + createContext: () => ({}), + onError: (options) => reportTRPCError(options.error, options.path), + }) +} + +afterEach(() => { + vi.clearAllMocks() +}) + +describe('tRPC server error reporting', () => { + it('reports a handled HTTP 500 with its original cause and no request input', async () => { + const error = new Error('Provider connection failed') + const response = await requestFailure(error) + + expect(response.status).toBe(500) + expect(mocks.error).toHaveBeenCalledExactlyOnceWith('[tRPC] Server error', error, { + procedure: 'probe', + code: 'INTERNAL_SERVER_ERROR', + }) + }) + + it.each([ + ['SERVICE_UNAVAILABLE', 503], + ['GATEWAY_TIMEOUT', 504], + ] as const)('reports %s responses', async (code, status) => { + const error = new TRPCError({ code }) + const response = await requestFailure(error) + + expect(response.status).toBe(status) + expect(mocks.error).toHaveBeenCalledExactlyOnceWith('[tRPC] Server error', error, { + procedure: 'probe', + code, + }) + }) + + it.each([ + ['BAD_REQUEST', 400], + ['UNAUTHORIZED', 401], + ['FORBIDDEN', 403], + ['NOT_FOUND', 404], + ['CONFLICT', 409], + ['TOO_MANY_REQUESTS', 429], + ] as const)('does not report expected %s responses', async (code, status) => { + const response = await requestFailure(new TRPCError({ code })) + + expect(response.status).toBe(status) + expect(mocks.error).not.toHaveBeenCalled() + }) +}) diff --git a/src/server/api/trpc-errors.ts b/src/server/api/trpc-errors.ts new file mode 100644 index 000000000..0cc96a473 --- /dev/null +++ b/src/server/api/trpc-errors.ts @@ -0,0 +1,12 @@ +import { type TRPCError } from '@trpc/server' +import { getHTTPStatusCodeFromError } from '@trpc/server/http' +import { logger } from '@/lib/logger' + +export function reportTRPCError(error: TRPCError, path: string | undefined): void { + if (getHTTPStatusCodeFromError(error) < 500) return + + logger.error('[tRPC] Server error', error.cause ?? error, { + procedure: path ?? '', + code: error.code, + }) +} From 9184240cf9da9382b25706785ff7eb87dd4501f4 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 00:32:07 +0100 Subject: [PATCH 02/10] fix: label Sentry releases and pin browser CI runner --- .github/workflows/e2e-tests.yml | 2 +- next.config.ts | 1 + src/sentry-build-config.test.ts | 18 ++++++++++++++++++ src/server/api/trpc-errors.test.ts | 16 ++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 src/sentry-build-config.test.ts diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 9d170853b..c03644ffa 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -29,7 +29,7 @@ concurrency: jobs: e2e-tests: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 timeout-minutes: 30 services: diff --git a/next.config.ts b/next.config.ts index b0e117116..c256fdf63 100644 --- a/next.config.ts +++ b/next.config.ts @@ -326,6 +326,7 @@ const analyzedConfig = withBundleAnalyzer(nextConfig) const sentryBuildOptions = { org: 'hexelnet', project: 'emuready', + release: { name: nextBuildId }, silent: !process.env.CI, widenClientFileUpload: true, sourcemaps: { diff --git a/src/sentry-build-config.test.ts b/src/sentry-build-config.test.ts new file mode 100644 index 000000000..17ca739f6 --- /dev/null +++ b/src/sentry-build-config.test.ts @@ -0,0 +1,18 @@ +import { afterEach, expect, it, vi } from 'vitest' + +afterEach(() => { + vi.unstubAllEnvs() + vi.resetModules() +}) + +it('injects the supplied build commit into client bundle configuration', async () => { + const revision = 'a'.repeat(40) + vi.stubEnv('NEXT_PUBLIC_ENABLE_SENTRY', 'true') + vi.stubEnv('NEXT_BUILD_ID', revision) + vi.stubEnv('VERCEL', '') + vi.stubEnv('SENTRY_RELEASE', '') + + const config = (await import('../next.config')).default + + expect(config).toHaveProperty('env._sentryRelease', revision) +}) diff --git a/src/server/api/trpc-errors.test.ts b/src/server/api/trpc-errors.test.ts index 11ff04895..f4c1fdca3 100644 --- a/src/server/api/trpc-errors.test.ts +++ b/src/server/api/trpc-errors.test.ts @@ -44,6 +44,22 @@ describe('tRPC server error reporting', () => { }) }) + it('reports a plain-object cause normalized by tRPC as an exception', async () => { + const error = new TRPCError({ + code: 'INTERNAL_SERVER_ERROR', + cause: { message: 'Provider connection failed', code: 'ECONNRESET' }, + }) + const response = await requestFailure(error) + + expect(response.status).toBe(500) + expect(error.cause).toBeInstanceOf(Error) + expect(error.cause?.message).toBe('Provider connection failed') + expect(mocks.error).toHaveBeenCalledExactlyOnceWith('[tRPC] Server error', error.cause, { + procedure: 'probe', + code: 'INTERNAL_SERVER_ERROR', + }) + }) + it.each([ ['SERVICE_UNAVAILABLE', 503], ['GATEWAY_TIMEOUT', 504], From 439d7dc2b46d4c94baf34f64fb24925d6b07898b Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 00:52:32 +0100 Subject: [PATCH 03/10] fix: disambiguate browser checks and preserve Sentry release fallback --- next.config.ts | 2 +- src/sentry-build-config.test.ts | 10 ++++++++++ tests/auth.spec.ts | 2 +- tests/error-handling.spec.ts | 6 ++++-- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/next.config.ts b/next.config.ts index c256fdf63..edaf7f3e8 100644 --- a/next.config.ts +++ b/next.config.ts @@ -326,7 +326,7 @@ const analyzedConfig = withBundleAnalyzer(nextConfig) const sentryBuildOptions = { org: 'hexelnet', project: 'emuready', - release: { name: nextBuildId }, + release: { name: nextBuildId || undefined }, silent: !process.env.CI, widenClientFileUpload: true, sourcemaps: { diff --git a/src/sentry-build-config.test.ts b/src/sentry-build-config.test.ts index 17ca739f6..6e210fe6e 100644 --- a/src/sentry-build-config.test.ts +++ b/src/sentry-build-config.test.ts @@ -16,3 +16,13 @@ it('injects the supplied build commit into client bundle configuration', async ( expect(config).toHaveProperty('env._sentryRelease', revision) }) + +it('preserves the SDK release fallback when the build identifier is empty', async () => { + vi.stubEnv('NEXT_PUBLIC_ENABLE_SENTRY', 'true') + vi.stubEnv('NEXT_BUILD_ID', '') + vi.stubEnv('SENTRY_RELEASE', 'fallback-release') + + const config = (await import('../next.config')).default + + expect(config).toHaveProperty('env._sentryRelease', 'fallback-release') +}) diff --git a/tests/auth.spec.ts b/tests/auth.spec.ts index a78e93409..a31fe780d 100644 --- a/tests/auth.spec.ts +++ b/tests/auth.spec.ts @@ -148,7 +148,7 @@ test.describe('Auth Error Handling', () => { // Clerk's hosted form uses input name="identifier" then "password" — these // input names are part of Clerk's public API. await page.locator('input[name="identifier"]').fill('invalid-e2e-test@example.com') - await page.getByRole('button', { name: /continue/i }).click() + await page.getByRole('button', { name: 'Continue', exact: true }).click() // role="alert" is Clerk's stable error contract; internal classes are not. await expect(page.getByRole('alert').first()).toBeVisible() diff --git a/tests/error-handling.spec.ts b/tests/error-handling.spec.ts index d1e5c589e..5dc762ad9 100644 --- a/tests/error-handling.spec.ts +++ b/tests/error-handling.spec.ts @@ -16,12 +16,14 @@ test.describe('Error Handling Tests', () => { await expect(page.getByText(/not found|does not exist|invalid|error/i).first()).toBeVisible() }) - test('should show error UI when API requests fail', async ({ page }) => { + test('should keep the games page shell usable when API requests fail', async ({ page }) => { await page.route('**/api/trpc/**', (route) => route.abort('failed')) await page.goto('/games', { waitUntil: 'domcontentloaded' }) - await expect(page.locator('main')).toBeVisible() + const gamesPage = new GamesPage(page) + await expect(gamesPage.pageHeading).toBeVisible() + await expect(gamesPage.searchInput).toBeEnabled() }) test('should render page shell even when API is slow', async ({ page }) => { From 7e968d07e2adc3da9eb9a61a8ae9f75c1214540d Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 01:06:11 +0100 Subject: [PATCH 04/10] test: wait for failed games request before checking page shell --- tests/error-handling.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/error-handling.spec.ts b/tests/error-handling.spec.ts index 5dc762ad9..6726bbb81 100644 --- a/tests/error-handling.spec.ts +++ b/tests/error-handling.spec.ts @@ -18,8 +18,13 @@ test.describe('Error Handling Tests', () => { test('should keep the games page shell usable when API requests fail', async ({ page }) => { await page.route('**/api/trpc/**', (route) => route.abort('failed')) + const failedGamesRequest = page.waitForEvent( + 'requestfailed', + (request) => request.url().includes('/api/trpc/') && request.url().includes('games.get'), + ) await page.goto('/games', { waitUntil: 'domcontentloaded' }) + await failedGamesRequest const gamesPage = new GamesPage(page) await expect(gamesPage.pageHeading).toBeVisible() From dbad987101fdbebb31f945ea22eadc93a571f9dd Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 03:58:16 +0100 Subject: [PATCH 05/10] fix: load server instrumentation from src and preserve error causes --- .github/workflows/ci.yml | 4 +++ sentry.server.config.ts | 2 +- src/instrumentation.test.ts | 2 +- instrumentation.ts => src/instrumentation.ts | 4 +-- src/lib/app-error-cause.test.ts | 18 +++++++++++ src/lib/errors.ts | 7 +++-- src/sentry-config.test.ts | 14 +++++++++ src/server/api/routers/admin/titleIdTools.ts | 10 ++++-- src/server/api/trpc-errors.test.ts | 12 +++++++ src/server/api/trpc-errors.ts | 7 ++++- .../repositories/base.repository.test.ts | 31 +++++++++++++++++++ src/server/repositories/base.repository.ts | 6 ++-- .../services/steam-batch-lookup.service.ts | 5 +-- 13 files changed, 107 insertions(+), 15 deletions(-) rename instrumentation.ts => src/instrumentation.ts (75%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93bd09782..97960ed65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -177,3 +177,7 @@ jobs: - name: Build run: pnpm build + + - name: Verify compiled server instrumentation + run: | + node -e 'const hook = require("./.next/server/instrumentation.js"); if (typeof hook.register !== "function" || typeof hook.onRequestError !== "function") process.exit(1)' diff --git a/sentry.server.config.ts b/sentry.server.config.ts index 398e66d98..b279a4b38 100644 --- a/sentry.server.config.ts +++ b/sentry.server.config.ts @@ -4,7 +4,7 @@ if (process.env.NEXT_PUBLIC_ENABLE_SENTRY === 'true') { Sentry.init({ dsn: 'https://85ca585e45005d8786e361c3456518bf@o74828.ingest.us.sentry.io/4509717207318529', environment: process.env.NEXT_PUBLIC_APP_ENV, - release: process.env.APP_VERSION, + ...(process.env.APP_VERSION ? { release: process.env.APP_VERSION } : {}), sampleRate: 1, tracesSampleRate: 0.1, enableLogs: true, diff --git a/src/instrumentation.test.ts b/src/instrumentation.test.ts index 62cbbbaed..c7acfeabc 100644 --- a/src/instrumentation.test.ts +++ b/src/instrumentation.test.ts @@ -23,7 +23,7 @@ async function loadInstrumentation(runtime: string, enabled = true) { vi.stubEnv('NEXT_RUNTIME', runtime) vi.stubEnv('NEXT_PUBLIC_ENABLE_SENTRY', String(enabled)) vi.stubEnv('NEXT_PHASE', '') - return import('../instrumentation') + return import('./instrumentation') } describe('server instrumentation entry point', () => { diff --git a/instrumentation.ts b/src/instrumentation.ts similarity index 75% rename from instrumentation.ts rename to src/instrumentation.ts index 399995591..d50c1e938 100644 --- a/instrumentation.ts +++ b/src/instrumentation.ts @@ -5,8 +5,8 @@ export async function register() { if (process.env.NEXT_PHASE === PHASE_PRODUCTION_BUILD) return if (process.env.NEXT_RUNTIME === 'nodejs') { - await import('./sentry.server.config') - const { initializeServerOnce } = await import('./src/server/init') + await import('../sentry.server.config') + const { initializeServerOnce } = await import('./server/init') await initializeServerOnce() } } diff --git a/src/lib/app-error-cause.test.ts b/src/lib/app-error-cause.test.ts index e5e15b513..2308c2c16 100644 --- a/src/lib/app-error-cause.test.ts +++ b/src/lib/app-error-cause.test.ts @@ -67,4 +67,22 @@ describe('getSerializableAppError', () => { expect(getSerializableAppError('nope')).toBeNull() expect(getSerializableAppError(42)).toBeNull() }) + + it.each(['internal', 'database'] as const)( + 'keeps %s diagnostics private while preserving the reported Error identity', + (kind) => { + const providerError = Object.assign(new Error('Provider failed'), { + code: 'PRIVATE_PROVIDER', + }) + const diagnosticError = new Error('Operation failed', { cause: providerError }) + const error = captureTrpcError(() => { + if (kind === 'database') AppError.databaseError('test operation', diagnosticError) + AppError.internalError('Operation failed', diagnosticError) + }) + + expect(error.cause).toBe(diagnosticError) + expect(diagnosticError.cause).toBe(providerError) + expect(roundTripAppError(error)).toBeNull() + }, + ) }) diff --git a/src/lib/errors.ts b/src/lib/errors.ts index d10e82874..cbad8bc0c 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -220,17 +220,18 @@ export class AppError { } // System errors - static internalError(message?: string): never { + static internalError(message?: string, cause?: Error): never { throw new TRPCError({ code: ERROR_CODES.INTERNAL_SERVER_ERROR, message: message ?? 'An internal server error occurred', + cause, }) } - static databaseError(operation?: string): never { + static databaseError(operation?: string, cause?: Error): never { const message = operation ? `Database error during ${operation}` : ERROR_MESSAGES.DATABASE_ERROR - throw new TRPCError({ code: ERROR_CODES.INTERNAL_SERVER_ERROR, message }) + throw new TRPCError({ code: ERROR_CODES.INTERNAL_SERVER_ERROR, message, cause }) } // Generic error thrower with custom code and message diff --git a/src/sentry-config.test.ts b/src/sentry-config.test.ts index 3dfac3fe5..855d8f062 100644 --- a/src/sentry-config.test.ts +++ b/src/sentry-config.test.ts @@ -24,6 +24,20 @@ afterEach(() => { }) describe('Sentry server config', () => { + it.each([undefined, ''])( + 'preserves the SDK release fallback when APP_VERSION is %s', + async (version) => { + vi.resetModules() + vi.stubEnv('NEXT_PUBLIC_ENABLE_SENTRY', 'true') + vi.stubEnv('APP_VERSION', version) + + await import('../sentry.server.config') + + expect(sentry.init).toHaveBeenCalledTimes(1) + expect(sentry.init.mock.calls[0]?.[0]).not.toHaveProperty('release') + }, + ) + it('does not initialize when the Sentry flag is disabled', async () => { await loadSentryConfigs({ NODE_ENV: 'production', diff --git a/src/server/api/routers/admin/titleIdTools.ts b/src/server/api/routers/admin/titleIdTools.ts index 389f9b96a..5a5d524df 100644 --- a/src/server/api/routers/admin/titleIdTools.ts +++ b/src/server/api/routers/admin/titleIdTools.ts @@ -46,8 +46,9 @@ export const titleIdToolsRouter = createTRPCRouter({ bestMatch, } } catch (error) { - logger.error('Title ID search failed', error) - return AppError.internalError('Failed to search for title IDs') + const diagnosticError = new Error('Title ID search failed', { cause: error }) + logger.error('Title ID search failed', diagnosticError) + return AppError.internalError('Failed to search for title IDs', diagnosticError) } }), @@ -66,7 +67,10 @@ export const titleIdToolsRouter = createTRPCRouter({ return await getTitleIdStats(input.platformId) } catch (error) { console.error('Title ID stats fetch failed', error) - return AppError.internalError('Failed to fetch title ID statistics') + return AppError.internalError( + 'Failed to fetch title ID statistics', + new Error('Title ID statistics failed', { cause: error }), + ) } }), diff --git a/src/server/api/trpc-errors.test.ts b/src/server/api/trpc-errors.test.ts index f4c1fdca3..221317a82 100644 --- a/src/server/api/trpc-errors.test.ts +++ b/src/server/api/trpc-errors.test.ts @@ -30,9 +30,21 @@ async function requestFailure(error: Error) { afterEach(() => { vi.clearAllMocks() + vi.restoreAllMocks() + vi.unstubAllEnvs() }) describe('tRPC server error reporting', () => { + it('keeps expected errors visible in development without sending them to Sentry', async () => { + vi.stubEnv('NODE_ENV', 'development') + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const response = await requestFailure(new TRPCError({ code: 'UNAUTHORIZED' })) + + expect(response.status).toBe(401) + expect(consoleError).toHaveBeenCalledExactlyOnceWith('tRPC failed on probe: UNAUTHORIZED') + expect(mocks.error).not.toHaveBeenCalled() + }) + it('reports a handled HTTP 500 with its original cause and no request input', async () => { const error = new Error('Provider connection failed') const response = await requestFailure(error) diff --git a/src/server/api/trpc-errors.ts b/src/server/api/trpc-errors.ts index 0cc96a473..9cea0a4c8 100644 --- a/src/server/api/trpc-errors.ts +++ b/src/server/api/trpc-errors.ts @@ -3,7 +3,12 @@ import { getHTTPStatusCodeFromError } from '@trpc/server/http' import { logger } from '@/lib/logger' export function reportTRPCError(error: TRPCError, path: string | undefined): void { - if (getHTTPStatusCodeFromError(error) < 500) return + if (getHTTPStatusCodeFromError(error) < 500) { + if (process.env.NODE_ENV === 'development') { + console.error(`tRPC failed on ${path ?? ''}: ${error.message}`) + } + return + } logger.error('[tRPC] Server error', error.cause ?? error, { procedure: path ?? '', diff --git a/src/server/repositories/base.repository.test.ts b/src/server/repositories/base.repository.test.ts index cbb1f90ce..67df04178 100644 --- a/src/server/repositories/base.repository.test.ts +++ b/src/server/repositories/base.repository.test.ts @@ -1,7 +1,12 @@ +import { TRPCError } from '@trpc/server' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { getSerializableAppError } from '@/lib/app-error-cause' import { type PrismaClient } from '@orm/client' import { BaseRepository } from './base.repository' +const mocks = vi.hoisted(() => ({ error: vi.fn() })) +vi.mock('@/lib/logger', () => ({ logger: { error: mocks.error } })) + vi.mock('@orm/client', async () => { const actual = await import('@orm/client') return { @@ -21,6 +26,10 @@ type BatchQueryFn = (params: { }) => Promise<{ id: string }[]> class TestRepository extends BaseRepository { + execute(operation: () => Promise) { + return this.handleDatabaseOperation(operation, 'test operation') + } + async *iterateBatches( queryFn: (params: { cursor?: { id: string }; take: number; skip?: number }) => Promise, batchSize?: number, @@ -37,9 +46,31 @@ describe('BaseRepository', () => { let repository: TestRepository beforeEach(() => { + vi.clearAllMocks() repository = new TestRepository(createMockPrisma()) }) + it('preserves the logged diagnostic cause without exposing provider metadata to clients', async () => { + const providerError = Object.assign(new Error('Connection failed'), { + code: 'ECONNRESET', + metadata: 'server-only diagnostic', + }) + try { + await repository.execute(() => Promise.reject(providerError)) + throw new Error('Expected the database operation to fail') + } catch (error) { + if (!(error instanceof TRPCError) || !error.cause) throw error + expect(error.message).toBe('Database error during test operation') + expect(error.cause.cause).toBe(providerError) + expect(getSerializableAppError(error.cause)).toBeNull() + expect(mocks.error).toHaveBeenCalledExactlyOnceWith( + '[BaseRepository] Database error', + error.cause, + { context: 'test operation' }, + ) + } + }) + describe('cursorBatchIterator', () => { it('should yield all batches correctly across multiple pages', async () => { const batch1 = [{ id: 'a1' }, { id: 'a2' }, { id: 'a3' }] diff --git a/src/server/repositories/base.repository.ts b/src/server/repositories/base.repository.ts index ad4836316..62d5c983b 100644 --- a/src/server/repositories/base.repository.ts +++ b/src/server/repositories/base.repository.ts @@ -91,8 +91,10 @@ export abstract class BaseRepository { throw error } // Generic database error for unknown errors - logger.error('[BaseRepository] Database error', error, { context }) - throw AppError.databaseError(context) + // Provider codes and metadata must not become a public appError payload. + const diagnosticError = new Error('Database operation failed', { cause: error }) + logger.error('[BaseRepository] Database error', diagnosticError, { context }) + throw AppError.databaseError(context, diagnosticError) } } diff --git a/src/server/services/steam-batch-lookup.service.ts b/src/server/services/steam-batch-lookup.service.ts index f51d11552..1c74911e9 100644 --- a/src/server/services/steam-batch-lookup.service.ts +++ b/src/server/services/steam-batch-lookup.service.ts @@ -112,7 +112,8 @@ export async function lookupGamesBySteamAppIds( } catch (error) { if (error instanceof TRPCError) throw error - logger.error('Error in batch Steam App ID lookup', error) - return AppError.internalError('Failed to lookup games by Steam App IDs') + const diagnosticError = new Error('Steam App ID lookup failed', { cause: error }) + logger.error('Error in batch Steam App ID lookup', diagnosticError) + return AppError.internalError('Failed to lookup games by Steam App IDs', diagnosticError) } } From 77f3b8a16bb877c58d702fbc263f02a559a9c3f7 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 05:34:54 +0100 Subject: [PATCH 06/10] fix: keep startup responsive and preserve server diagnostics --- sentry.server.config.ts | 3 +- src/lib/app-error-cause.test.ts | 29 ++++++++ src/lib/app-error-cause.ts | 7 +- src/sentry-config.test.ts | 10 +++ src/server/api/routers/admin/titleIdTools.ts | 8 +-- src/server/init.test.ts | 66 +++++++++++++++++++ src/server/init.ts | 27 ++++---- .../repositories/base.repository.test.ts | 24 ++++++- src/server/repositories/base.repository.ts | 2 - 9 files changed, 150 insertions(+), 26 deletions(-) create mode 100644 src/server/init.test.ts diff --git a/sentry.server.config.ts b/sentry.server.config.ts index b279a4b38..36cd652f1 100644 --- a/sentry.server.config.ts +++ b/sentry.server.config.ts @@ -1,9 +1,10 @@ import * as Sentry from '@sentry/nextjs' +import { env } from '@/lib/env' if (process.env.NEXT_PUBLIC_ENABLE_SENTRY === 'true') { Sentry.init({ dsn: 'https://85ca585e45005d8786e361c3456518bf@o74828.ingest.us.sentry.io/4509717207318529', - environment: process.env.NEXT_PUBLIC_APP_ENV, + environment: env.APP_ENV, ...(process.env.APP_VERSION ? { release: process.env.APP_VERSION } : {}), sampleRate: 1, tracesSampleRate: 0.1, diff --git a/src/lib/app-error-cause.test.ts b/src/lib/app-error-cause.test.ts index 2308c2c16..863b17ba6 100644 --- a/src/lib/app-error-cause.test.ts +++ b/src/lib/app-error-cause.test.ts @@ -68,6 +68,35 @@ describe('getSerializableAppError', () => { expect(getSerializableAppError(42)).toBeNull() }) + it.each(['internal', 'database'] as const)( + 'does not serialize raw provider errors passed to %s failures', + (kind) => { + const providerError = Object.assign(new Error('Provider failed'), { + code: 'P2022', + meta: { column: 'private_column' }, + }) + const error = captureTrpcError(() => { + if (kind === 'database') AppError.databaseError('test operation', providerError) + AppError.internalError('Operation failed', providerError) + }) + + expect(error.cause).toBe(providerError) + expect(roundTripAppError(error)).toBeNull() + }, + ) + + it('preserves submission-limit details for the client', () => { + const error = captureTrpcError(() => ResourceError.game.submissionLimitExceeded(10, 3, 2)) + + expect(roundTripAppError(error)).toEqual({ + code: APP_ERROR_CODES.GAME_SUBMISSION_LIMIT_EXCEEDED, + gamesCount: 10, + listingsCount: 3, + buffer: 2, + reportsNeeded: 6, + }) + }) + it.each(['internal', 'database'] as const)( 'keeps %s diagnostics private while preserving the reported Error identity', (kind) => { diff --git a/src/lib/app-error-cause.ts b/src/lib/app-error-cause.ts index b8cddb991..f9caf2201 100644 --- a/src/lib/app-error-cause.ts +++ b/src/lib/app-error-cause.ts @@ -1,7 +1,8 @@ +import { APP_ERROR_CODES } from './errors' + export function getSerializableAppError(cause: unknown): Record | null { - if (!cause || typeof cause !== 'object') return null + if (!cause || typeof cause !== 'object' || !('code' in cause)) return null - const payload = cause as Record // tRPC wraps `cause` in an Error and superjson strips custom props off Errors; copy to a plain object. - return typeof payload.code === 'string' ? { ...payload } : null + return Object.values(APP_ERROR_CODES).some((code) => code === cause.code) ? { ...cause } : null } diff --git a/src/sentry-config.test.ts b/src/sentry-config.test.ts index 855d8f062..ad18a3fce 100644 --- a/src/sentry-config.test.ts +++ b/src/sentry-config.test.ts @@ -65,4 +65,14 @@ describe('Sentry server config', () => { }), ) }) + + it('uses the shared environment fallback for an invalid app environment', async () => { + await loadSentryConfigs({ + NODE_ENV: 'production', + NEXT_PUBLIC_APP_ENV: 'invalid', + NEXT_PUBLIC_ENABLE_SENTRY: 'true', + }) + + expect(sentry.init).toHaveBeenCalledWith(expect.objectContaining({ environment: 'local' })) + }) }) diff --git a/src/server/api/routers/admin/titleIdTools.ts b/src/server/api/routers/admin/titleIdTools.ts index 5a5d524df..0acaa132f 100644 --- a/src/server/api/routers/admin/titleIdTools.ts +++ b/src/server/api/routers/admin/titleIdTools.ts @@ -66,11 +66,9 @@ export const titleIdToolsRouter = createTRPCRouter({ try { return await getTitleIdStats(input.platformId) } catch (error) { - console.error('Title ID stats fetch failed', error) - return AppError.internalError( - 'Failed to fetch title ID statistics', - new Error('Title ID statistics failed', { cause: error }), - ) + const diagnosticError = new Error('Title ID statistics failed', { cause: error }) + logger.error('Title ID stats fetch failed', diagnosticError) + return AppError.internalError('Failed to fetch title ID statistics', diagnosticError) } }), diff --git a/src/server/init.test.ts b/src/server/init.test.ts new file mode 100644 index 000000000..4f2991c03 --- /dev/null +++ b/src/server/init.test.ts @@ -0,0 +1,66 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + notification: vi.fn(), + switchGame: vi.fn<() => Promise>(), + threeDsGame: vi.fn<() => Promise>(), + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})) + +vi.mock('@/lib/logger', () => ({ logger: mocks.logger })) +vi.mock('./notifications/init', () => ({ initializeNotificationService: mocks.notification })) +vi.mock('./utils/switchGameInit', () => ({ initializeSwitchGameService: mocks.switchGame })) +vi.mock('./utils/threeDsGameInit', () => ({ initializeThreeDsGameService: mocks.threeDsGame })) + +beforeEach(() => { + vi.resetModules() + vi.resetAllMocks() + Reflect.deleteProperty(globalThis, 'initializationPromise') + Reflect.deleteProperty(globalThis, 'serviceStatus') +}) + +afterEach(() => { + Reflect.deleteProperty(globalThis, 'initializationPromise') + Reflect.deleteProperty(globalThis, 'serviceStatus') +}) + +describe('server startup', () => { + it('registers notification listeners once without waiting for stalled catalog downloads', async () => { + mocks.switchGame.mockReturnValue(new Promise(() => {})) + mocks.threeDsGame.mockReturnValue(new Promise(() => {})) + const server = await import('./init') + + await server.initializeServerOnce() + await server.initializeServerOnce() + + expect(mocks.notification).toHaveBeenCalledTimes(1) + expect(mocks.switchGame).toHaveBeenCalledTimes(1) + expect(mocks.threeDsGame).toHaveBeenCalledTimes(1) + expect(server.getServiceStatus()).toMatchObject({ + notification: { initialized: true }, + switchGame: { initialized: false }, + threeDsGame: { initialized: false }, + }) + }) + + it('records catalog startup failures without rejecting server startup', async () => { + const upstreamError = new Error('Catalog unavailable') + mocks.switchGame.mockRejectedValue(upstreamError) + mocks.threeDsGame.mockResolvedValue() + const server = await import('./init') + + await server.initializeServerOnce() + + await vi.waitFor(() => { + expect(mocks.logger.warn).toHaveBeenCalledWith( + '⚠️ Some services failed to initialize. They will be retried on next access.', + ) + }) + expect(mocks.notification).toHaveBeenCalledTimes(1) + expect(mocks.logger.error).toHaveBeenCalledWith(expect.any(String), upstreamError) + expect(server.getServiceStatus()).toMatchObject({ + switchGame: { initialized: false, failureCount: 1 }, + threeDsGame: { initialized: true }, + }) + }) +}) diff --git a/src/server/init.ts b/src/server/init.ts index a055a33af..39b07e2a7 100644 --- a/src/server/init.ts +++ b/src/server/init.ts @@ -162,20 +162,19 @@ async function initThreeDsGameServiceWithRetry(): Promise { export async function initializeServer(): Promise { logger.info('🚀 Initializing server services...') - // Initialize services in parallel with individual error handling - const [notificationSuccess, switchGameSuccess, threeDsGameSuccess] = await Promise.all([ - initNotificationServiceWithRetry(), - initSwitchGameServiceWithRetry(), - initThreeDsGameServiceWithRetry(), - ]) - - // Log overall status - const allSuccess = notificationSuccess && switchGameSuccess && threeDsGameSuccess - if (allSuccess) { - logger.info('✅ All server services initialized successfully') - } else { - logger.warn('⚠️ Some services failed to initialize. They will be retried on next access.') - } + const notificationSuccess = await initNotificationServiceWithRetry() + + // Notification listeners must be registered before requests; catalog warmups can finish later. + void Promise.all([initSwitchGameServiceWithRetry(), initThreeDsGameServiceWithRetry()]) + .then(([switchGameSuccess, threeDsGameSuccess]) => { + const allSuccess = notificationSuccess && switchGameSuccess && threeDsGameSuccess + if (allSuccess) { + logger.info('✅ All server services initialized successfully') + } else { + logger.warn('⚠️ Some services failed to initialize. They will be retried on next access.') + } + }) + .catch((error: unknown) => logger.error('[init] Failed to initialize catalog services', error)) } /** diff --git a/src/server/repositories/base.repository.test.ts b/src/server/repositories/base.repository.test.ts index 67df04178..2eae5c125 100644 --- a/src/server/repositories/base.repository.test.ts +++ b/src/server/repositories/base.repository.test.ts @@ -1,7 +1,7 @@ import { TRPCError } from '@trpc/server' import { beforeEach, describe, expect, it, vi } from 'vitest' import { getSerializableAppError } from '@/lib/app-error-cause' -import { type PrismaClient } from '@orm/client' +import { Prisma, type PrismaClient } from '@orm/client' import { BaseRepository } from './base.repository' const mocks = vi.hoisted(() => ({ error: vi.fn() })) @@ -71,6 +71,28 @@ describe('BaseRepository', () => { } }) + it('preserves unmapped Prisma failures for reporting without exposing their metadata', async () => { + const providerError = new Prisma.PrismaClientKnownRequestError('Column missing', { + code: 'P2022', + clientVersion: 'test', + meta: { column: 'private_column' }, + }) + try { + await repository.execute(() => Promise.reject(providerError)) + throw new Error('Expected the database operation to fail') + } catch (error) { + if (!(error instanceof TRPCError) || !error.cause) throw error + expect(error.message).toBe('Database error during test operation') + expect(error.cause.cause).toBe(providerError) + expect(getSerializableAppError(error.cause)).toBeNull() + expect(mocks.error).toHaveBeenCalledExactlyOnceWith( + '[BaseRepository] Database error', + error.cause, + { context: 'test operation' }, + ) + } + }) + describe('cursorBatchIterator', () => { it('should yield all batches correctly across multiple pages', async () => { const batch1 = [{ id: 'a1' }, { id: 'a2' }, { id: 'a3' }] diff --git a/src/server/repositories/base.repository.ts b/src/server/repositories/base.repository.ts index 62d5c983b..ad4bf140d 100644 --- a/src/server/repositories/base.repository.ts +++ b/src/server/repositories/base.repository.ts @@ -74,8 +74,6 @@ export abstract class BaseRepository { throw AppError.notFound(context) case 'P2003': // Foreign key constraint violation throw AppError.resourceInUse(context || 'Resource') - default: - throw AppError.databaseError(context) } } // Re-throw if it's already an AppError or ResourceError From 5d97182d21694d4bbe723292108ae58b5855c91b Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 05:35:35 +0100 Subject: [PATCH 07/10] test: wait for report table before checking accessibility --- tests/accessibility.spec.ts | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/accessibility.spec.ts b/tests/accessibility.spec.ts index dd6cc211e..7eec08d11 100644 --- a/tests/accessibility.spec.ts +++ b/tests/accessibility.spec.ts @@ -224,19 +224,17 @@ test.describe('Accessibility Tests', () => { const listingsPage = new ListingsPage(page) await listingsPage.goto() - const tables = page.locator('table') - const tableCount = await tables.count() - expect(tableCount).toBeGreaterThan(0) - - const headerData = await tables - .first() - .locator('th') - .evaluateAll((elements) => - elements.map((el) => ({ - scope: el.getAttribute('scope'), - text: el.textContent, - })), - ) + const table = page.getByRole('table').first() + await expect(table).toBeVisible() + const headers = table.locator('th') + await expect(headers.first()).toBeVisible() + + const headerData = await headers.evaluateAll((elements) => + elements.map((el) => ({ + scope: el.getAttribute('scope'), + text: el.textContent, + })), + ) expect(headerData.length).toBeGreaterThan(0) From b338e804e215157f1d4dccefc4f1d8d51945fe0d Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 05:52:33 +0100 Subject: [PATCH 08/10] fix: share catalog loads during startup and concurrent lookups --- src/server/utils/switchGameSearch.test.ts | 51 ++++++++++++++++++++++ src/server/utils/switchGameSearch.ts | 28 +++++++----- src/server/utils/threeDsGameSearch.test.ts | 26 +++++++++++ src/server/utils/threeDsGameSearch.ts | 28 +++++++----- 4 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 src/server/utils/switchGameSearch.test.ts diff --git a/src/server/utils/switchGameSearch.test.ts b/src/server/utils/switchGameSearch.test.ts new file mode 100644 index 000000000..5f0ec5fee --- /dev/null +++ b/src/server/utils/switchGameSearch.test.ts @@ -0,0 +1,51 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const games = [ + { program_id: '01006A800016E000', name: 'Test Adventure', title_normalized: 'test adventure' }, +] + +const fetchMock = vi.fn() + +beforeEach(() => { + vi.resetModules() + fetchMock.mockReset() + vi.stubGlobal('fetch', fetchMock) +}) + +afterEach(() => { + vi.unstubAllGlobals() + vi.restoreAllMocks() +}) + +describe('Switch catalog loading', () => { + it('shares one download between a warmup and concurrent lookups', async () => { + fetchMock.mockImplementation(async () => new Response(JSON.stringify(games))) + const catalog = await import('./switchGameSearch') + + const [, first, second, stats] = await Promise.all([ + catalog.refreshSwitchGamesData(), + catalog.findTitleIdForGameName('Test Adventure'), + catalog.findTitleIdForGameName('Test Adventure'), + catalog.getSwitchGamesStats(), + ]) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(first[0]?.titleId).toBe(games[0].program_id) + expect(second).toEqual(first) + expect(stats.totalGames).toBe(1) + }) + + it('allows another download after a failed load', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + fetchMock + .mockRejectedValueOnce(new Error('Upstream unavailable')) + .mockResolvedValueOnce(new Response(JSON.stringify(games))) + const catalog = await import('./switchGameSearch') + + await expect(catalog.refreshSwitchGamesData()).rejects.toThrow('Upstream unavailable') + const results = await catalog.findTitleIdForGameName('Test Adventure') + + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(results[0]?.titleId).toBe(games[0].program_id) + }) +}) diff --git a/src/server/utils/switchGameSearch.ts b/src/server/utils/switchGameSearch.ts index fa260a6ab..88b6a40bb 100644 --- a/src/server/utils/switchGameSearch.ts +++ b/src/server/utils/switchGameSearch.ts @@ -45,6 +45,7 @@ const FUSE_OPTIONS = { } const SWITCH_GAMES_URL = 'https://producdevity.github.io/switch-games-json/switchbrew_id_names.json' +let switchGamesDataPromise: Promise | null = null async function fetchSwitchGamesData(): Promise { const response = await fetch(SWITCH_GAMES_URL, { @@ -76,16 +77,21 @@ async function getSwitchGamesData(): Promise { const cachedData = switchGamesDataCache.get(cacheKey) if (cachedData) return cachedData.data - try { - const freshData = await fetchSwitchGamesData() - switchGamesDataCache.set(cacheKey, { data: freshData, createdAt: new Date() }) - return freshData - } catch (error) { - if (error instanceof Error) { - error.message = `Switch games data fetch failed: ${error.message}` - } - throw error - } + switchGamesDataPromise ??= fetchSwitchGamesData() + .then((freshData) => { + switchGamesDataCache.set(cacheKey, { data: freshData, createdAt: new Date() }) + return freshData + }) + .catch((error: unknown) => { + if (error instanceof Error) { + error.message = `Switch games data fetch failed: ${error.message}` + } + throw error + }) + .finally(() => { + switchGamesDataPromise = null + }) + return switchGamesDataPromise } async function getFuseInstance(): Promise> { @@ -95,6 +101,8 @@ async function getFuseInstance(): Promise> { if (cachedFuse) return cachedFuse const gamesData = await getSwitchGamesData() + const readyFuse = switchGamesFuseCache.get(cacheKey) + if (readyFuse) return readyFuse const fuse = new Fuse(gamesData, FUSE_OPTIONS) switchGamesFuseCache.set(cacheKey, fuse) diff --git a/src/server/utils/threeDsGameSearch.test.ts b/src/server/utils/threeDsGameSearch.test.ts index d0632adf6..19fe65f0b 100644 --- a/src/server/utils/threeDsGameSearch.test.ts +++ b/src/server/utils/threeDsGameSearch.test.ts @@ -3,6 +3,8 @@ import { clearThreeDsCachesForTests, findThreeDsTitleIdForGameName, getBestThreeDsTitleIdMatch, + getThreeDsGamesStats, + refreshThreeDsGamesData, } from './threeDsGameSearch' const TITLES_URL = 'https://dantheman827.github.io/nus-info/titles.json' @@ -109,6 +111,30 @@ describe('threeDsGameSearch', () => { expect(fetchMock).toHaveBeenCalledTimes(2) }) + it('shares the two dataset downloads between warmup and concurrent lookups', async () => { + const [, first, second, stats] = await Promise.all([ + refreshThreeDsGamesData(), + findThreeDsTitleIdForGameName('Mario Party Star Rush'), + findThreeDsTitleIdForGameName('Mario Party Star Rush'), + getThreeDsGamesStats(), + ]) + + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(first[0]?.titleId).toBe('00040000001A5F00') + expect(second).toEqual(first) + expect(stats.totalGames).toBe(2) + }) + + it('allows another download after a failed load', async () => { + fetchMock.mockRejectedValueOnce(new Error('Upstream unavailable')) + + await expect(refreshThreeDsGamesData()).rejects.toThrow('Upstream unavailable') + const results = await findThreeDsTitleIdForGameName('Mario Party Star Rush') + + expect(fetchMock).toHaveBeenCalledTimes(4) + expect(results[0]?.titleId).toBe('00040000001A5F00') + }) + it('filters out non-3DS platform entries', async () => { const results = await findThreeDsTitleIdForGameName('Tears of the Kingdom') expect(results).toHaveLength(0) diff --git a/src/server/utils/threeDsGameSearch.ts b/src/server/utils/threeDsGameSearch.ts index 7f8e110a6..dada051de 100644 --- a/src/server/utils/threeDsGameSearch.ts +++ b/src/server/utils/threeDsGameSearch.ts @@ -77,6 +77,7 @@ const FUSE_OPTIONS: IFuseOptions = { const THREEDS_TITLES_URL = 'https://dantheman827.github.io/nus-info/titles.json' const THREEDS_TITLE_NAMES_URL = 'https://dantheman827.github.io/nus-info/title-names.json' +let threeDsGamesDataPromise: Promise | null = null const threeDsGamesDataCache = new LRUCache>({ ttl: CACHE_DURATIONS.STATIC, @@ -207,16 +208,21 @@ async function getThreeDsGamesData(): Promise { const cached = threeDsGamesDataCache.get(cacheKey) if (cached) return cached.data - try { - const fresh = await fetchThreeDsGamesData() - threeDsGamesDataCache.set(cacheKey, { data: fresh, createdAt: new Date() }) - return fresh - } catch (error) { - if (error instanceof Error) { - error.message = `3DS games data fetch failed: ${error.message}` - } - throw error - } + threeDsGamesDataPromise ??= fetchThreeDsGamesData() + .then((fresh) => { + threeDsGamesDataCache.set(cacheKey, { data: fresh, createdAt: new Date() }) + return fresh + }) + .catch((error: unknown) => { + if (error instanceof Error) { + error.message = `3DS games data fetch failed: ${error.message}` + } + throw error + }) + .finally(() => { + threeDsGamesDataPromise = null + }) + return threeDsGamesDataPromise } async function getFuseInstance(): Promise> { @@ -225,6 +231,8 @@ async function getFuseInstance(): Promise> { if (cached) return cached const games = await getThreeDsGamesData() + const readyFuse = threeDsGamesFuseCache.get(cacheKey) + if (readyFuse) return readyFuse const fuse = new Fuse(games, FUSE_OPTIONS) threeDsGamesFuseCache.set(cacheKey, fuse) return fuse From 5292700ebcc5f3a009b1c76409c44d559c0a2cdc Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 05:59:25 +0100 Subject: [PATCH 09/10] test: verify catalog outage recovery and tRPC route reporting --- src/server/api/trpc-routes.test.ts | 70 +++++++++++++++++++++++++ src/server/catalog-startup.test.ts | 83 ++++++++++++++++++++++++++++++ src/server/init.test.ts | 2 +- 3 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 src/server/api/trpc-routes.test.ts create mode 100644 src/server/catalog-startup.test.ts diff --git a/src/server/api/trpc-routes.test.ts b/src/server/api/trpc-routes.test.ts new file mode 100644 index 000000000..edf221f3b --- /dev/null +++ b/src/server/api/trpc-routes.test.ts @@ -0,0 +1,70 @@ +import { TRPCError } from '@trpc/server' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { GET as mobileGET } from '@/app/api/mobile/trpc/[trpc]/route' +import { GET as webGET } from '@/app/api/trpc/[trpc]/route' +import type * as NextServer from 'next/server' + +const mocks = vi.hoisted(() => ({ + error: new Error('Provider unavailable'), + loggerError: vi.fn(), + connection: vi.fn(async () => {}), +})) + +vi.mock('next/server', async (importOriginal) => ({ + ...(await importOriginal()), + connection: mocks.connection, +})) +vi.mock('@/lib/logger', () => ({ logger: { error: mocks.loggerError } })) +vi.mock('@/server/api/trpc', () => ({ createAppRouterTRPCContext: () => ({}) })) +vi.mock('@/server/api/mobileContext', () => ({ createMobileTRPCFetchContext: () => ({}) })) +vi.mock('@/server/api/root', async () => { + const trpc = (await import('@trpc/server')).initTRPC.create() + return { + appRouter: trpc.router({ + probe: trpc.procedure.query(() => { + throw mocks.error + }), + }), + } +}) +vi.mock('@/server/api/routers/mobile', async () => { + const trpc = (await import('@trpc/server')).initTRPC.create() + return { + mobileRouter: trpc.router({ + probe: trpc.procedure.query(() => { + throw mocks.error + }), + }), + } +}) + +beforeEach(() => { + vi.clearAllMocks() + mocks.error = new Error('Provider unavailable') +}) + +describe.each([ + { name: 'web', get: webGET, endpoint: '/api/trpc' }, + { name: 'mobile', get: mobileGET, endpoint: '/api/mobile/trpc' }, +])('$name tRPC route reporting', (route) => { + it('reports handled 500 responses through the exported GET handler', async () => { + const response = await route.get(new NextRequest(`https://example.test${route.endpoint}/probe`)) + + expect(response.status).toBe(500) + expect(mocks.connection).toHaveBeenCalledTimes(1) + expect(mocks.loggerError).toHaveBeenCalledExactlyOnceWith('[tRPC] Server error', mocks.error, { + procedure: 'probe', + code: 'INTERNAL_SERVER_ERROR', + }) + }) + + it('excludes expected 401 responses from server-error reporting', async () => { + mocks.error = new TRPCError({ code: 'UNAUTHORIZED' }) + + const response = await route.get(new NextRequest(`https://example.test${route.endpoint}/probe`)) + + expect(response.status).toBe(401) + expect(mocks.loggerError).not.toHaveBeenCalled() + }) +}) diff --git a/src/server/catalog-startup.test.ts b/src/server/catalog-startup.test.ts new file mode 100644 index 000000000..f99b21bcd --- /dev/null +++ b/src/server/catalog-startup.test.ts @@ -0,0 +1,83 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ms } from '@/utils/time' + +const mocks = vi.hoisted(() => ({ + notification: vi.fn(), + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), log: vi.fn() }, +})) + +vi.mock('@/lib/logger', () => ({ logger: mocks.logger })) +vi.mock('./notifications/init', () => ({ initializeNotificationService: mocks.notification })) + +beforeEach(() => { + vi.resetModules() + vi.resetAllMocks() + vi.useFakeTimers() + vi.stubEnv('NODE_ENV', 'production') + vi.stubEnv('CI', '') + vi.stubEnv('PWTEST_SKIP_WEBSERVER', '') + vi.stubEnv('PLAYWRIGHT_TEST', '') + vi.spyOn(console, 'log').mockImplementation(() => {}) + vi.spyOn(console, 'error').mockImplementation(() => {}) + Reflect.deleteProperty(globalThis, 'initializationPromise') + Reflect.deleteProperty(globalThis, 'serviceStatus') +}) + +afterEach(() => { + vi.clearAllTimers() + vi.useRealTimers() + vi.unstubAllEnvs() + vi.unstubAllGlobals() + vi.restoreAllMocks() + Reflect.deleteProperty(globalThis, 'initializationPromise') + Reflect.deleteProperty(globalThis, 'serviceStatus') +}) + +describe('real catalog startup', () => { + it('keeps refresh services scheduled after a provider outage and recovers on the timer', async () => { + const fetchMock = vi.fn().mockRejectedValue(new Error('Provider unavailable')) + vi.stubGlobal('fetch', fetchMock) + const server = await import('./init') + + await server.initializeServerOnce() + await vi.waitFor(() => { + expect(server.getServiceStatus()).toMatchObject({ + notification: { initialized: true }, + switchGame: { initialized: true }, + threeDsGame: { initialized: true }, + }) + }) + expect(fetchMock).toHaveBeenCalled() + expect(mocks.notification).toHaveBeenCalledTimes(1) + + fetchMock.mockImplementation(async (input) => { + const url = String(input) + const titleId = '00040000001A5F00' + if (url.endsWith('switchbrew_id_names.json')) { + return Response.json([ + { + program_id: '01006A800016E000', + name: 'Test Adventure', + title_normalized: 'test adventure', + }, + ]) + } + if (url.endsWith('/titles.json')) { + return Response.json({ [titleId]: { title_id: titleId, platform_device: 'CTR' } }) + } + return Response.json({ [titleId]: { US: 'Test Adventure' } }) + }) + await vi.advanceTimersByTimeAsync(ms.hours(24)) + + const switchCatalog = await import('./utils/switchGameSearch') + const threeDsCatalog = await import('./utils/threeDsGameSearch') + expect(await switchCatalog.getSwitchGamesStats()).toMatchObject({ + totalGames: 1, + cacheStatus: 'hit', + }) + expect(await threeDsCatalog.getThreeDsGamesStats()).toMatchObject({ + totalGames: 1, + cacheStatus: 'hit', + }) + }) +}) diff --git a/src/server/init.test.ts b/src/server/init.test.ts index 4f2991c03..4919f2ece 100644 --- a/src/server/init.test.ts +++ b/src/server/init.test.ts @@ -43,7 +43,7 @@ describe('server startup', () => { }) }) - it('records catalog startup failures without rejecting server startup', async () => { + it('records an initializer rejection without rejecting server startup', async () => { const upstreamError = new Error('Catalog unavailable') mocks.switchGame.mockRejectedValue(upstreamError) mocks.threeDsGame.mockResolvedValue() From 56b01c0a464d86082ac1ab2d454dbb40bb04ad44 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 07:59:42 +0100 Subject: [PATCH 10/10] fix: isolate catalog refreshes and bound shared downloads --- src/server/utils/switchGameSearch.test.ts | 85 ++++++++++++++++++ src/server/utils/switchGameSearch.ts | 27 +++--- src/server/utils/threeDsGameSearch.test.ts | 100 +++++++++++++++++++++ src/server/utils/threeDsGameSearch.ts | 38 +++++--- 4 files changed, 225 insertions(+), 25 deletions(-) diff --git a/src/server/utils/switchGameSearch.test.ts b/src/server/utils/switchGameSearch.test.ts index 5f0ec5fee..4862f28c4 100644 --- a/src/server/utils/switchGameSearch.test.ts +++ b/src/server/utils/switchGameSearch.test.ts @@ -18,6 +18,91 @@ afterEach(() => { }) describe('Switch catalog loading', () => { + it.each(['headers', 'body'])( + 'releases a load stalled while reading %s after its deadline', + async (phase) => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + const controller = new AbortController() + const deadline = new DOMException('Catalog deadline', 'TimeoutError') + const timeout = vi.spyOn(AbortSignal, 'timeout').mockReturnValue(controller.signal) + const network = Promise.withResolvers() + fetchMock.mockImplementation(async (_input, options) => { + const signal = options?.signal ?? controller.signal + if (phase === 'body') { + return new Response( + new ReadableStream({ + start(stream) { + signal.addEventListener('abort', () => stream.error(signal.reason), { once: true }) + }, + }), + ) + } + signal.addEventListener('abort', () => network.reject(signal.reason), { once: true }) + return network.promise + }) + const catalog = await import('./switchGameSearch') + const load = catalog.refreshSwitchGamesData().catch((error: unknown) => error) + + try { + expect(timeout).toHaveBeenCalledWith(30_000) + expect(fetchMock.mock.calls[0]?.[1]?.signal).toBe(controller.signal) + controller.abort(deadline) + expect(await load).toBe(deadline) + timeout.mockRestore() + fetchMock.mockResolvedValue(Response.json(games)) + expect((await catalog.findTitleIdForGameName('Test Adventure'))[0]?.titleId).toBe( + games[0].program_id, + ) + } finally { + controller.abort(new Error('Test cleanup')) + if (phase === 'headers') network.reject(new Error('Test cleanup')) + await load + } + }, + ) + + it.each([true, false])( + 'keeps refreshed data when the older lookup finishes first: %s', + async (oldFirst) => { + const old = Promise.withResolvers() + const fresh = Promise.withResolvers() + const freshGames = [ + { ...games[0], program_id: '01006A800016E001' }, + { program_id: '01006A800016E002', name: 'Other Game', title_normalized: 'other game' }, + ] + fetchMock.mockReturnValueOnce(old.promise).mockReturnValueOnce(fresh.promise) + const catalog = await import('./switchGameSearch') + const lookup = catalog.findTitleIdForGameName('Test Adventure') + const refresh = catalog.refreshSwitchGamesData() + + try { + expect(fetchMock).toHaveBeenCalledTimes(2) + if (oldFirst) { + old.resolve(Response.json(games)) + await lookup + const concurrent = catalog.findTitleIdForGameName('Test Adventure') + expect(fetchMock).toHaveBeenCalledTimes(2) + fresh.resolve(Response.json(freshGames)) + await Promise.all([refresh, concurrent]) + } else { + fresh.resolve(Response.json(freshGames)) + await refresh + old.resolve(Response.json(games)) + await lookup + } + expect((await catalog.findTitleIdForGameName('Test Adventure'))[0]?.titleId).toBe( + freshGames[0].program_id, + ) + expect((await catalog.getSwitchGamesStats()).totalGames).toBe(2) + expect(fetchMock).toHaveBeenCalledTimes(2) + } finally { + old.resolve(Response.json(games)) + fresh.resolve(Response.json(freshGames)) + await Promise.allSettled([lookup, refresh]) + } + }, + ) + it('shares one download between a warmup and concurrent lookups', async () => { fetchMock.mockImplementation(async () => new Response(JSON.stringify(games))) const catalog = await import('./switchGameSearch') diff --git a/src/server/utils/switchGameSearch.ts b/src/server/utils/switchGameSearch.ts index 88b6a40bb..84979be85 100644 --- a/src/server/utils/switchGameSearch.ts +++ b/src/server/utils/switchGameSearch.ts @@ -45,11 +45,14 @@ const FUSE_OPTIONS = { } const SWITCH_GAMES_URL = 'https://producdevity.github.io/switch-games-json/switchbrew_id_names.json' +const FETCH_TIMEOUT_MS = 30_000 let switchGamesDataPromise: Promise | null = null +let cacheGeneration = 0 async function fetchSwitchGamesData(): Promise { const response = await fetch(SWITCH_GAMES_URL, { headers: { 'User-Agent': 'EmuReady-GameSearch/1.0' }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }) if (!response.ok) { @@ -77,21 +80,20 @@ async function getSwitchGamesData(): Promise { const cachedData = switchGamesDataCache.get(cacheKey) if (cachedData) return cachedData.data - switchGamesDataPromise ??= fetchSwitchGamesData() + if (switchGamesDataPromise) return switchGamesDataPromise + const generation = cacheGeneration + const load: ReturnType = fetchSwitchGamesData() .then((freshData) => { - switchGamesDataCache.set(cacheKey, { data: freshData, createdAt: new Date() }) - return freshData - }) - .catch((error: unknown) => { - if (error instanceof Error) { - error.message = `Switch games data fetch failed: ${error.message}` + if (generation === cacheGeneration) { + switchGamesDataCache.set(cacheKey, { data: freshData, createdAt: new Date() }) } - throw error + return freshData }) .finally(() => { - switchGamesDataPromise = null + if (switchGamesDataPromise === load) switchGamesDataPromise = null }) - return switchGamesDataPromise + switchGamesDataPromise = load + return load } async function getFuseInstance(): Promise> { @@ -100,12 +102,13 @@ async function getFuseInstance(): Promise> { const cachedFuse = switchGamesFuseCache.get(cacheKey) if (cachedFuse) return cachedFuse + const generation = cacheGeneration const gamesData = await getSwitchGamesData() const readyFuse = switchGamesFuseCache.get(cacheKey) if (readyFuse) return readyFuse const fuse = new Fuse(gamesData, FUSE_OPTIONS) - switchGamesFuseCache.set(cacheKey, fuse) + if (generation === cacheGeneration) switchGamesFuseCache.set(cacheKey, fuse) return fuse } @@ -196,6 +199,8 @@ export async function getBestTitleIdMatch(gameName: string): Promise { try { + cacheGeneration++ + switchGamesDataPromise = null switchGamesDataCache.clear() switchGamesFuseCache.clear() diff --git a/src/server/utils/threeDsGameSearch.test.ts b/src/server/utils/threeDsGameSearch.test.ts index 19fe65f0b..0dbdbfb44 100644 --- a/src/server/utils/threeDsGameSearch.test.ts +++ b/src/server/utils/threeDsGameSearch.test.ts @@ -87,6 +87,106 @@ describe('threeDsGameSearch', () => { afterEach(() => { clearThreeDsCachesForTests() global.fetch = originalFetch + vi.restoreAllMocks() + }) + + it.each(['headers', 'body'])( + 'releases a load stalled while reading %s after its deadline', + async (phase) => { + const controller = new AbortController() + const deadline = new DOMException('Catalog deadline', 'TimeoutError') + const timeout = vi.spyOn(AbortSignal, 'timeout').mockReturnValue(controller.signal) + const network = Promise.withResolvers() + fetchMock.mockImplementation(async (_input, options) => { + const signal = options?.signal ?? controller.signal + if (phase === 'body') { + return new Response( + new ReadableStream({ + start(stream) { + signal.addEventListener('abort', () => stream.error(signal.reason), { once: true }) + }, + }), + ) + } + signal.addEventListener('abort', () => network.reject(signal.reason), { once: true }) + return network.promise + }) + const load = refreshThreeDsGamesData().catch((error: unknown) => error) + + try { + expect(timeout).toHaveBeenCalledWith(30_000) + expect(fetchMock.mock.calls[0]?.[1]?.signal).toBe(controller.signal) + expect(fetchMock.mock.calls[1]?.[1]?.signal).toBe(controller.signal) + controller.abort(deadline) + expect(await load).toBe(deadline) + timeout.mockRestore() + fetchMock.mockImplementation(createFetchMock()) + expect((await findThreeDsTitleIdForGameName('Mario Party Star Rush'))[0]?.titleId).toBe( + '00040000001A5F00', + ) + } finally { + controller.abort(new Error('Test cleanup')) + if (phase === 'headers') network.reject(new Error('Test cleanup')) + await load + } + }, + ) + + it.each([ + { operation: 'refresh', oldFirst: true }, + { operation: 'refresh', oldFirst: false }, + { operation: 'clear', oldFirst: false }, + ])('keeps refreshed data after $operation with oldFirst=$oldFirst', async (options) => { + const oldTitles = Promise.withResolvers() + const oldNames = Promise.withResolvers() + const freshTitles = Promise.withResolvers() + const freshNames = Promise.withResolvers() + const freshId = '00040000001A5F01' + const titles = { [freshId]: { title_id: freshId, platform_device: 'CTR' } } + const names = { [freshId]: { US: 'Mario Party Star Rush' } } + fetchMock + .mockReturnValueOnce(oldTitles.promise) + .mockReturnValueOnce(oldNames.promise) + .mockReturnValueOnce(freshTitles.promise) + .mockReturnValueOnce(freshNames.promise) + const lookup = findThreeDsTitleIdForGameName('Mario Party Star Rush') + if (options.operation === 'clear') clearThreeDsCachesForTests() + const reload = + options.operation === 'refresh' + ? refreshThreeDsGamesData() + : findThreeDsTitleIdForGameName('Mario Party Star Rush') + + try { + expect(fetchMock).toHaveBeenCalledTimes(4) + if (options.oldFirst) { + oldTitles.resolve(createJsonResponse(sampleTitles)) + oldNames.resolve(createJsonResponse(sampleNames)) + await lookup + const concurrent = findThreeDsTitleIdForGameName('Mario Party Star Rush') + expect(fetchMock).toHaveBeenCalledTimes(4) + freshTitles.resolve(createJsonResponse(titles)) + freshNames.resolve(createJsonResponse(names)) + await Promise.all([reload, concurrent]) + } else { + freshTitles.resolve(createJsonResponse(titles)) + freshNames.resolve(createJsonResponse(names)) + await reload + oldTitles.resolve(createJsonResponse(sampleTitles)) + oldNames.resolve(createJsonResponse(sampleNames)) + await lookup + } + expect((await findThreeDsTitleIdForGameName('Mario Party Star Rush'))[0]?.titleId).toBe( + freshId, + ) + expect((await getThreeDsGamesStats()).totalGames).toBe(1) + expect(fetchMock).toHaveBeenCalledTimes(4) + } finally { + oldTitles.resolve(createJsonResponse(sampleTitles)) + oldNames.resolve(createJsonResponse(sampleNames)) + freshTitles.resolve(createJsonResponse(titles)) + freshNames.resolve(createJsonResponse(names)) + await Promise.allSettled([lookup, reload]) + } }) it('finds title IDs for matching Nintendo 3DS games', async () => { diff --git a/src/server/utils/threeDsGameSearch.ts b/src/server/utils/threeDsGameSearch.ts index dada051de..dff03e281 100644 --- a/src/server/utils/threeDsGameSearch.ts +++ b/src/server/utils/threeDsGameSearch.ts @@ -77,7 +77,9 @@ const FUSE_OPTIONS: IFuseOptions = { const THREEDS_TITLES_URL = 'https://dantheman827.github.io/nus-info/titles.json' const THREEDS_TITLE_NAMES_URL = 'https://dantheman827.github.io/nus-info/title-names.json' +const FETCH_TIMEOUT_MS = 30_000 let threeDsGamesDataPromise: Promise | null = null +let cacheGeneration = 0 const threeDsGamesDataCache = new LRUCache>({ ttl: CACHE_DURATIONS.STATIC, @@ -137,12 +139,15 @@ function pickPreferredName( } async function fetchThreeDsGamesData(): Promise { + const signal = AbortSignal.timeout(FETCH_TIMEOUT_MS) const [titlesResponse, titleNamesResponse] = await Promise.all([ fetch(THREEDS_TITLES_URL, { headers: { 'User-Agent': 'EmuReady-3DSLookup/1.0' }, + signal, }), fetch(THREEDS_TITLE_NAMES_URL, { headers: { 'User-Agent': 'EmuReady-3DSLookup/1.0' }, + signal, }), ]) @@ -208,21 +213,20 @@ async function getThreeDsGamesData(): Promise { const cached = threeDsGamesDataCache.get(cacheKey) if (cached) return cached.data - threeDsGamesDataPromise ??= fetchThreeDsGamesData() + if (threeDsGamesDataPromise) return threeDsGamesDataPromise + const generation = cacheGeneration + const load: ReturnType = fetchThreeDsGamesData() .then((fresh) => { - threeDsGamesDataCache.set(cacheKey, { data: fresh, createdAt: new Date() }) - return fresh - }) - .catch((error: unknown) => { - if (error instanceof Error) { - error.message = `3DS games data fetch failed: ${error.message}` + if (generation === cacheGeneration) { + threeDsGamesDataCache.set(cacheKey, { data: fresh, createdAt: new Date() }) } - throw error + return fresh }) .finally(() => { - threeDsGamesDataPromise = null + if (threeDsGamesDataPromise === load) threeDsGamesDataPromise = null }) - return threeDsGamesDataPromise + threeDsGamesDataPromise = load + return load } async function getFuseInstance(): Promise> { @@ -230,11 +234,12 @@ async function getFuseInstance(): Promise> { const cached = threeDsGamesFuseCache.get(cacheKey) if (cached) return cached + const generation = cacheGeneration const games = await getThreeDsGamesData() const readyFuse = threeDsGamesFuseCache.get(cacheKey) if (readyFuse) return readyFuse const fuse = new Fuse(games, FUSE_OPTIONS) - threeDsGamesFuseCache.set(cacheKey, fuse) + if (generation === cacheGeneration) threeDsGamesFuseCache.set(cacheKey, fuse) return fuse } @@ -351,8 +356,7 @@ export async function getBestThreeDsTitleIdMatch(gameName: string): Promise { - threeDsGamesDataCache.clear() - threeDsGamesFuseCache.clear() + clearThreeDsCaches() await getThreeDsGamesData() await getFuseInstance() } @@ -389,7 +393,13 @@ export async function getThreeDsGamesStats(): Promise<{ } } -export function clearThreeDsCachesForTests(): void { +function clearThreeDsCaches(): void { + cacheGeneration++ + threeDsGamesDataPromise = null threeDsGamesDataCache.clear() threeDsGamesFuseCache.clear() } + +export function clearThreeDsCachesForTests(): void { + clearThreeDsCaches() +}