From fe1dae8b5f9a2c717630392e43e27f1bb9f9767c Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 30 Oct 2026 23:24:08 +0100 Subject: [PATCH 01/16] 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/16] 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/16] 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/16] 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 512a5e818bf5aa7739f11802f45c4e2912518b82 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 00:56:21 +0100 Subject: [PATCH 05/16] ci: deploy branch images after all checks pass --- .github/workflows/ci.yml | 29 +++- .github/workflows/container-image.yml | 33 ++++- .github/workflows/e2e-tests.yml | 26 +--- docs/SELF_HOSTING.md | 14 +- docs/container-image.md | 56 ++++++-- scripts/container_deploy.py | 189 ++++++++++++++++++++++++++ scripts/container_deploy_test.py | 169 +++++++++++++++++++++++ 7 files changed, 471 insertions(+), 45 deletions(-) create mode 100644 scripts/container_deploy.py create mode 100644 scripts/container_deploy_test.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93bd09782..d20cb2862 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ permissions: concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true + cancel-in-progress: ${{ github.event_name == 'pull_request' }} env: DATABASE_URL: "postgresql://postgres:postgres@localhost:5432/emuready_test" @@ -60,6 +60,9 @@ jobs: - name: Test container build configuration run: python3 -m unittest discover -s scripts -p container_build_config_test.py + - name: Test release safety checks + run: python3 -m unittest discover -s scripts -p container_deploy_test.py + - name: Setup pnpm uses: pnpm/action-setup@v6 @@ -74,7 +77,7 @@ jobs: run: pnpm install --frozen-lockfile - name: Run database migrations - run: pnpm exec prisma migrate deploy + run: pnpm db:migrate:deploy - name: Generate Prisma client run: pnpm db:generate @@ -122,7 +125,7 @@ jobs: run: pnpm install --frozen-lockfile - name: Run database migrations - run: pnpm exec prisma migrate deploy + run: pnpm db:migrate:deploy - name: Generate Prisma client run: pnpm db:generate @@ -170,10 +173,28 @@ jobs: run: pnpm clean - name: Run database migrations - run: pnpm exec prisma migrate deploy + run: pnpm db:migrate:deploy - name: Generate Prisma client run: pnpm db:generate - name: Build run: pnpm build + + browser-tests: + name: Browser tests + uses: ./.github/workflows/e2e-tests.yml + secrets: inherit + + release: + name: Publish and deploy + if: github.event_name == 'push' && (github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/master') + needs: [quality, unit-tests, build, browser-tests] + permissions: + contents: read + packages: write + uses: ./.github/workflows/container-image.yml + with: + publish: true + deploy: true + secrets: inherit diff --git a/.github/workflows/container-image.yml b/.github/workflows/container-image.yml index d15ff05a5..2abdc11c7 100644 --- a/.github/workflows/container-image.yml +++ b/.github/workflows/container-image.yml @@ -1,6 +1,14 @@ name: Container image on: + workflow_call: + inputs: + publish: + type: boolean + default: false + deploy: + type: boolean + default: false workflow_dispatch: inputs: publish: @@ -45,6 +53,18 @@ jobs: - uses: actions/checkout@v6 with: persist-credentials: false + fetch-depth: 0 + + - name: Check release target and database compatibility + if: inputs.deploy + env: + COOLIFY_TOKEN: ${{ secrets.COOLIFY_TOKEN }} + COOLIFY_APPLICATION_UUID: ${{ vars.COOLIFY_APPLICATION_UUID }} + CONTAINER_APPROVED_SCHEMA_DIGEST: ${{ vars.CONTAINER_APPROVED_SCHEMA_DIGEST }} + PUBLISH_IMAGE: ${{ inputs.publish }} + run: | + test "$PUBLISH_IMAGE" = true + python3 scripts/container_deploy.py --preflight - uses: pnpm/action-setup@v6 @@ -87,6 +107,7 @@ jobs: printf 'name=%s\ntag=%s\n' "$image" "$tag" >> "$GITHUB_OUTPUT" - name: Publish image + id: publish if: inputs.publish env: GHCR_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -98,4 +119,14 @@ jobs: docker push "$IMAGE:$TAG" digest=$(docker buildx imagetools inspect "$IMAGE:$TAG" --format '{{json .Manifest}}' | jq -er '.digest') [[ "$digest" =~ ^sha256:[a-f0-9]{64}$ ]] - printf 'Deploy this immutable image through Coolify Docker Image:\n\n`%s`\n' "$IMAGE@$digest" >> "$GITHUB_STEP_SUMMARY" + printf 'image=%s\n' "$IMAGE@$digest" >> "$GITHUB_OUTPUT" + printf 'Published immutable image:\n\n %s\n' "$IMAGE@$digest" >> "$GITHUB_STEP_SUMMARY" + + - name: Deploy and verify release + if: inputs.deploy + env: + COOLIFY_TOKEN: ${{ secrets.COOLIFY_TOKEN }} + COOLIFY_APPLICATION_UUID: ${{ vars.COOLIFY_APPLICATION_UUID }} + CONTAINER_APPROVED_SCHEMA_DIGEST: ${{ vars.CONTAINER_APPROVED_SCHEMA_DIGEST }} + RELEASE_IMAGE: ${{ steps.publish.outputs.image }} + run: python3 scripts/container_deploy.py --image "$RELEASE_IMAGE" diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index c03644ffa..a06607af4 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -1,32 +1,12 @@ name: E2E Tests on: + workflow_call: workflow_dispatch: - pull_request: - paths: - - "src/**" - - "tests/**" - - "playwright.config.ts" - - "package.json" - - "pnpm-lock.yaml" - - "pnpm-workspace.yaml" - push: - branches: [master, staging] - paths: - - "src/**" - - "tests/**" - - "playwright.config.ts" - - "package.json" - - "pnpm-lock.yaml" - - "pnpm-workspace.yaml" permissions: contents: read -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - jobs: e2e-tests: runs-on: ubuntu-24.04 @@ -87,13 +67,13 @@ jobs: run: pnpm install --frozen-lockfile - name: Run database migrations - run: pnpm exec prisma migrate deploy + run: pnpm db:migrate:deploy - name: Generate Prisma client run: pnpm db:generate - name: Seed test database - run: pnpm exec prisma db seed + run: pnpm db:seed - name: Build application run: pnpm build diff --git a/docs/SELF_HOSTING.md b/docs/SELF_HOSTING.md index df5366e8c..186509658 100644 --- a/docs/SELF_HOSTING.md +++ b/docs/SELF_HOSTING.md @@ -9,17 +9,19 @@ EmuReady runs as a standalone Next.js container behind Coolify and Cloudflare. S - Set `BUILD_DATABASE_URL` to a migrated, disposable Postgres database because Prisma TypedSQL generation introspects the schema. Never use production or a restored production backup for this. Apply the existing migrations to an empty database; seeds are unnecessary. - Optionally set `BUILD_DATABASE_DIRECT_URL` for that same disposable database. It defaults to `BUILD_DATABASE_URL`. The builder maps these names to Prisma's `DATABASE_URL` and `DATABASE_DIRECT_URL` only for `pnpm build`, and fails if `BUILD_DATABASE_URL` is missing. - Keep the real `DATABASE_URL`, `DATABASE_DIRECT_URL`, and other runtime-only secrets, such as `CLERK_SECRET_KEY`, out of the build phase. Backups and environment files must remain excluded from the Docker context. -- For a release containing migrations, build the `migrator` target from the same commit and run it with `DATABASE_DIRECT_URL` before deploying the `app` image. +- For a release containing migrations, verify a fresh backup and obtain explicit approval, then build the `migrator` target from the same commit and run it with `DATABASE_DIRECT_URL` before deploying the `app` image. -Use the manual [Container image workflow](container-image.md) to build and publish -images on GitHub runners, then deploy their immutable digests through Coolify's -Docker Image build pack. Disable source auto-deployment to keep builds off the -application VPS. Image publication does not deploy or apply hosted migrations. +CI calls the [Container image workflow](container-image.md) after quality and +browser checks pass, then deploys its immutable digest through Coolify's Docker +Image build pack. `staging` and `master` select separate GitHub environments and +Coolify applications. Keep Coolify source auto-deployment disabled so builds stay +on GitHub runners. Manual image publication remains available; hosted migrations +require a separate approved operation. ## Coolify application - Use the Docker Image build pack with the verified immutable digest and exposed port `3000`. -- Set runtime `APP_VERSION` to the exact source commit. The image workflow sets `NEXT_BUILD_ID` while building. +- Use the image's embedded `APP_VERSION`; disable Coolify runtime overrides for `APP_VERSION` and `NEXT_BUILD_ID`. The image workflow supplies the exact source commit during the build. - Use `/api/health/ready` for deployment health checks and `/api/health/live` for process liveness. ## Production configuration diff --git a/docs/container-image.md b/docs/container-image.md index f450fda27..d818b439f 100644 --- a/docs/container-image.md +++ b/docs/container-image.md @@ -1,7 +1,14 @@ # Container images -The manual **Container image** workflow builds on GitHub runners and optionally -publishes to private GHCR. It does not deploy or change DNS. +Merges into `staging` and `master` run CI, including browser tests. Once all jobs +pass, CI calls **Container image** to build on GitHub runners, publish to private +GHCR, and deploy the immutable digest through Coolify. `staging` deploys to +staging; `master` deploys to production. DNS and hosted databases are not changed. + +Manual dispatch still builds and optionally publishes an image, with `publish` +defaulting to false. Automatic deployment is called from CI after all checks pass. +A branch that has advanced since the workflow started is rejected before +deployment. A running release is not cancelled by a newer push. Create a GitHub Actions environment **variable** named `CONTAINER_PUBLIC_BUILD_ENV` containing a JSON object in each environment below. The workflow reads it through @@ -19,14 +26,36 @@ application secrets: the workflow migrates an empty PostgreSQL 15 database on th Prisma generation. Public settings are baked into the image and require a rebuild when changed. -After the exact commit passes CI, dispatch the workflow on the corresponding -branch with `publish` enabled. Before deployment, check the target database's -applied migrations against that commit. This workflow migrates only its disposable -database and publishes only the `app` target; the application does not migrate on -startup. If migrations are pending, stop until a fresh backup is verified and the -user explicitly approves applying them. Build the same-commit `migrator` target -off the application VPS and apply the migrations as described in -[Self-hosting](SELF_HOSTING.md#build-and-release-contract) before deploying the app. +Each environment also needs a `COOLIFY_APPLICATION_UUID` variable and a +`COOLIFY_TOKEN` secret. Use a team-scoped token with `read`, `write`, and `deploy` +permissions. It inspects the non-sensitive application configuration, selects the +image digest and source commit, triggers the deployment and reads its status. +It does not need `root`, `read:sensitive` or `write:sensitive`. Store it only in +the matching branch-restricted GitHub environment and rotate it before expiry. + +The deployment script refuses source build packs, the wrong branch/image +repository, missing readiness checks and pre/post deployment commands. Disable +runtime overrides for `APP_VERSION` and `NEXT_BUILD_ID` in Coolify: the image +already embeds its source commit. The public `/api/health` version and readiness +must match after Coolify finishes. A queued request alone is not success. + +The schema and migration files are compared with the current public deployment's +source commit. Any difference blocks deployment. This workflow migrates only its +disposable runner database; the application does not migrate on startup. + +For a schema change, verify a fresh backup and obtain explicit approval before +applying hosted migrations. Build the same-commit `migrator` target off the +application VPS and follow +[Self-hosting](SELF_HOSTING.md#build-and-release-contract). After the migration +is verified, set the environment variable `CONTAINER_APPROVED_SCHEMA_DIGEST` to +the exact digest printed by: + +```sh +GITHUB_SHA="$(git rev-parse HEAD)" python3 scripts/container_deploy.py --schema-digest +``` + +This approval matches only that schema/migration file set; it does not authorize +future changes. Never set it before the backup, approval and migration checks. Before deployment, configure GHCR authentication for the Docker account used by Coolify on every deployment server. Use a pull-only credential with `read:packages` @@ -38,7 +67,12 @@ Deploy that immutable digest through Coolify's Docker Image build pack. Preserve runtime credentials, domains, labels, port 3000, health checks, certificate mounts and persistent storage. Disable source auto-deployment so pushes cannot start builds on the application VPS. -Set runtime `APP_VERSION` to the image's exact source commit. +The image supplies `APP_VERSION` from the exact build commit. + +If a release fails, inspect its deployment UUID and workflow logs before retrying. +Coolify keeps the previous container when the new container fails readiness. +The workflow prints the previous image selection for manual rollback; it does +not apply database changes or initiate an extra rollback deployment. Verify image identity, readiness, TLS, canonical URLs, authentication and static assets before switching public DNS. Keep the existing Vercel deployments and diff --git a/scripts/container_deploy.py b/scripts/container_deploy.py new file mode 100644 index 000000000..5011811dc --- /dev/null +++ b/scripts/container_deploy.py @@ -0,0 +1,189 @@ +import argparse +import hashlib +import json +import os +from pathlib import Path +import re +import subprocess +import time + + +COOLIFY_URL = 'https://coolify.emuready.com/api/v1' +TARGETS = { + 'refs/heads/master': 'https://www.emuready.com', + 'refs/heads/staging': 'https://staging.emuready.com', +} +IMAGE_NAME = 'ghcr.io/producdevity/emuready' + + +class ReleaseError(Exception): + pass + + +class Client: + def __init__(self, base, token=None): + self.base = base + self.token = token + + def request(self, path, method='GET', payload=None): + config = [f'url = {json.dumps(self.base + path)}', f'request = {json.dumps(method)}'] + config.append('header = "Accept: application/json"') + if self.token: + if any(char in self.token for char in '\r\n\0'): + raise ReleaseError('Invalid Coolify token format') + config.append(f'header = {json.dumps("Authorization: Bearer " + self.token)}') + if payload is not None: + config.extend(['header = "Content-Type: application/json"', + f'data = {json.dumps(json.dumps(payload))}']) + result = subprocess.run( + ['curl', '--disable', '--config', '-', '--silent', '--show-error', '--connect-timeout', '10', + '--max-time', '30', '--proto', '=https', '--write-out', '\n%{http_code}'], + input='\n'.join(config), text=True, capture_output=True, + ) + body, _, status = result.stdout.rpartition('\n') + accepted_statuses = ('200', '201') if method == 'PATCH' else ('200',) + if result.returncode or status not in accepted_statuses: + raise ReleaseError(f'{method} request failed (HTTP {status or "unknown"})') + try: + return json.loads(body) + except json.JSONDecodeError as error: + raise ReleaseError('Expected a JSON response') from error + + +def git(*arguments): + result = subprocess.run(['git', *arguments], capture_output=True, check=False) + if result.returncode: + raise ReleaseError('Unable to inspect release history or branch head') + return result.stdout + + +def schema_digest(revision): + if not isinstance(revision, str) or not re.fullmatch('[a-f0-9]{40}', revision): + raise ReleaseError('Expected an immutable source commit') + paths = git('ls-tree', '-r', '-z', '--name-only', revision, '--', + 'prisma/schema.prisma', 'prisma/migrations').split(b'\0') + paths = sorted(path for path in paths if path) + if b'prisma/schema.prisma' not in paths: + raise ReleaseError('Release history does not contain the database schema') + digest = hashlib.sha256() + for path in paths: + content = git('show', f'{revision}:{path.decode()}') + for value in (path, content): + digest.update(len(value).to_bytes(8, 'big')) + digest.update(value) + return digest.hexdigest() + + +def preflight(ref, revision, application_uuid, api, public, approved_schema=None): + if ref not in TARGETS or not re.fullmatch('[a-f0-9]{40}', revision): + raise ReleaseError('Releases require an exact staging or master commit') + if not re.fullmatch('[a-zA-Z0-9]{10,64}', application_uuid): + raise ReleaseError('Set the target environment application UUID') + remote = git('ls-remote', '--exit-code', 'origin', ref).decode().split() + if len(remote) != 2 or remote[0] != revision or remote[1] != ref: + raise ReleaseError('Branch advanced; refusing to deploy an outdated release') + application = api.request(f'/applications/{application_uuid}') + domains = application.get('fqdn', '') + if not isinstance(domains, str) or TARGETS[ref] not in {url.rstrip('/') for url in domains.split(',')}: + raise ReleaseError('Coolify application domains do not match the selected environment') + if (application.get('uuid') != application_uuid + or application.get('git_branch') != ref.removeprefix('refs/heads/') + or application.get('build_pack') != 'dockerimage' + or application.get('docker_registry_image_name') != IMAGE_NAME + or not re.fullmatch('sha256-[a-f0-9]{64}', application.get('docker_registry_image_tag', '')) + or application.get('health_check_path') != '/api/health/ready' + or not application.get('health_check_enabled') + or application.get('pre_deployment_command') + or application.get('post_deployment_command')): + raise ReleaseError('Coolify target does not match the guarded image deployment') + variables = api.request(f'/applications/{application_uuid}/envs') + if any(variable.get('key') in ('APP_VERSION', 'NEXT_BUILD_ID') and variable.get('is_runtime') + for variable in variables): + raise ReleaseError('Remove runtime version overrides so the image supplies its own commit') + deployed = public.request('/api/health') + if deployed.get('status') != 'healthy': + raise ReleaseError('The current application is not healthy; investigate before releasing') + desired_schema = schema_digest(revision) + if schema_digest(deployed.get('version', '')) != desired_schema and approved_schema != desired_schema: + raise ReleaseError('Database changes require an approved backup and migration before deployment') + return application + + +def deploy(ref, revision, application_uuid, image, api, public, approved_schema=None): + if not re.fullmatch(re.escape(IMAGE_NAME) + r'@sha256:[a-f0-9]{64}', image): + raise ReleaseError('Only an immutable EmuReady image digest may be deployed') + application = preflight(ref, revision, application_uuid, api, public, approved_schema) + previous_image = f'{IMAGE_NAME}@sha256:{application["docker_registry_image_tag"].removeprefix("sha256-")}' + print(f'Previous image: {previous_image}', flush=True) + # Coolify restricts the deploy API's docker_tag override to preview deployments. + api.request(f'/applications/{application_uuid}', 'PATCH', { + 'docker_registry_image_tag': image.split('@')[1].replace('sha256:', 'sha256-'), + 'git_commit_sha': revision, + }) + queued = api.request('/deploy', 'POST', {'uuid': application_uuid, 'force': False}) + deployments = queued.get('deployments', []) + if (len(deployments) != 1 or deployments[0].get('resource_uuid') != application_uuid + or not re.fullmatch('[a-zA-Z0-9]{10,64}', deployments[0].get('deployment_uuid', ''))): + raise ReleaseError('Coolify did not queue the requested application deployment') + deployment_uuid = deployments[0]['deployment_uuid'] + print(f'Deployment {deployment_uuid}', flush=True) + deadline = time.monotonic() + 900 + while time.monotonic() < deadline: + deployment = api.request(f'/deployments/{deployment_uuid}') + status = deployment.get('status') + if status == 'finished': + break + if status not in ('queued', 'in_progress'): + raise ReleaseError(f'Coolify deployment ended with status {status}') + time.sleep(10) + else: + raise ReleaseError('Timed out waiting for Coolify; inspect the deployment before retrying') + for attempt in range(6): + try: + health = public.request('/api/health') + ready = public.request('/api/health/ready') + if health.get('version') == revision and health.get('status') == 'healthy' and ready.get('status') == 'healthy': + return deployment_uuid + except ReleaseError: + pass + if attempt < 5: + time.sleep(10) + raise ReleaseError('Deployment finished, but public readiness and release version did not match') + + +def main(): + parser = argparse.ArgumentParser() + mode = parser.add_mutually_exclusive_group(required=True) + mode.add_argument('--preflight', action='store_true') + mode.add_argument('--image') + mode.add_argument('--schema-digest', action='store_true') + arguments = parser.parse_args() + ref = os.environ.get('GITHUB_REF', '') + revision = os.environ.get('GITHUB_SHA', '') + if arguments.schema_digest: + print(schema_digest(revision)) + return + token = os.environ.get('COOLIFY_TOKEN', '') + if ref not in TARGETS or not token: + raise ReleaseError('Set the branch-scoped Coolify deployment credential') + api = Client(COOLIFY_URL, token) + public = Client(TARGETS[ref]) + application_uuid = os.environ.get('COOLIFY_APPLICATION_UUID', '') + approved_schema = os.environ.get('CONTAINER_APPROVED_SCHEMA_DIGEST') + if arguments.preflight: + preflight(ref, revision, application_uuid, api, public, approved_schema) + print('Release target, branch head and database compatibility verified') + else: + deployment_uuid = deploy(ref, revision, application_uuid, arguments.image, api, public, approved_schema) + summary = f'\nDeployed `{revision}` as `{arguments.image}` through Coolify `{deployment_uuid}`.\n' + if os.environ.get('GITHUB_STEP_SUMMARY'): + with Path(os.environ['GITHUB_STEP_SUMMARY']).open('a') as output: + output.write(summary) + print(summary.strip()) + + +if __name__ == '__main__': + try: + main() + except ReleaseError as error: + raise SystemExit(str(error)) from error diff --git a/scripts/container_deploy_test.py b/scripts/container_deploy_test.py new file mode 100644 index 000000000..cfbec9d8a --- /dev/null +++ b/scripts/container_deploy_test.py @@ -0,0 +1,169 @@ +import subprocess +import unittest +from unittest.mock import patch + +import container_deploy as release + + +REF = 'refs/heads/staging' +OLD = '1' * 40 +REVISION = '2' * 40 +UUID = 'jzfkmcmpz0xat15arcayharg' +DEPLOYMENT = 'testdeployment1234567890' +IMAGE = release.IMAGE_NAME + '@sha256:' + '3' * 64 +SCHEMA_DIGEST = release.schema_digest + + +class FakeApi: + def __init__(self): + self.application = { + 'uuid': UUID, + 'git_branch': 'staging', + 'fqdn': 'https://staging.emuready.com', + 'build_pack': 'dockerimage', + 'docker_registry_image_name': release.IMAGE_NAME, + 'docker_registry_image_tag': 'sha256-' + '4' * 64, + 'health_check_path': '/api/health/ready', + 'health_check_enabled': True, + } + self.variables = [] + self.status = 'finished' + self.calls = [] + + def request(self, path, method='GET', payload=None): + self.calls.append((path, method, payload)) + if method == 'PATCH': + return {} + if path == '/deploy': + return {'deployments': [{'resource_uuid': UUID, 'deployment_uuid': DEPLOYMENT}]} + if path == '/deployments/' + DEPLOYMENT: + return {'status': self.status} + if path.endswith('/envs'): + return self.variables + return self.application + + +class FakePublic: + def __init__(self): + self.version = REVISION + self.calls = 0 + + def request(self, path): + if path == '/api/health/ready': + return {'status': 'healthy'} + self.calls += 1 + return {'status': 'healthy', 'version': OLD if self.calls == 1 else self.version} + + +class DeploymentTests(unittest.TestCase): + def setUp(self): + self.api = FakeApi() + self.public = FakePublic() + self.branch = patch.object(release, 'git', return_value=f'{REVISION}\t{REF}\n'.encode()).start() + self.schema = patch.object(release, 'schema_digest', return_value='a' * 64).start() + patch.object(release.time, 'sleep').start() + self.addCleanup(patch.stopall) + + def deploy(self, image=IMAGE, approved_schema=None): + return release.deploy(REF, REVISION, UUID, image, self.api, self.public, approved_schema) + + def assert_no_writes(self): + self.assertTrue(all(method == 'GET' for _, method, _ in self.api.calls)) + + def test_deploys_exact_digest_only_after_preflight_and_checks_public_version(self): + self.assertEqual(self.deploy(), DEPLOYMENT) + self.assertEqual(self.api.calls[2], ('/applications/' + UUID, 'PATCH', { + 'docker_registry_image_tag': 'sha256-' + '3' * 64, + 'git_commit_sha': REVISION, + })) + self.assertEqual(self.api.calls[3], ('/deploy', 'POST', {'uuid': UUID, 'force': False})) + self.assertEqual(self.public.calls, 2) + + def test_rejects_mutable_tags_before_contacting_coolify(self): + with self.assertRaisesRegex(release.ReleaseError, 'immutable'): + self.deploy(release.IMAGE_NAME + ':latest') + self.assertEqual(self.api.calls, []) + + def test_rejects_a_stale_build_without_mutation(self): + self.branch.return_value = f'{OLD}\t{REF}\n'.encode() + with self.assertRaisesRegex(release.ReleaseError, 'Branch advanced'): + self.deploy() + self.assertEqual(self.api.calls, []) + + def test_rejects_wrong_environment_without_mutation(self): + self.api.application['git_branch'] = 'master' + with self.assertRaisesRegex(release.ReleaseError, 'target'): + self.deploy() + self.assert_no_writes() + + def test_rejects_deployment_commands_without_mutation(self): + self.api.application['pre_deployment_command'] = 'pnpm db:migrate:deploy' + with self.assertRaisesRegex(release.ReleaseError, 'target'): + self.deploy() + self.assert_no_writes() + + def test_rejects_the_wrong_public_domain_without_mutation(self): + self.api.application['fqdn'] = 'https://www.emuready.com' + with self.assertRaisesRegex(release.ReleaseError, 'domains'): + self.deploy() + self.assert_no_writes() + + def test_master_uses_the_production_application(self): + ref = 'refs/heads/master' + self.branch.return_value = f'{REVISION}\t{ref}\n'.encode() + self.api.application.update(git_branch='master', fqdn='https://www.emuready.com,https://emuready.com') + self.assertEqual(release.deploy(ref, REVISION, UUID, IMAGE, self.api, self.public), DEPLOYMENT) + + def test_rejects_an_old_runtime_version_override(self): + self.api.variables = [{'key': 'APP_VERSION', 'is_runtime': True}] + with self.assertRaisesRegex(release.ReleaseError, 'version overrides'): + self.deploy() + self.assert_no_writes() + + def test_schema_changes_require_an_exact_approved_digest(self): + self.schema.side_effect = lambda revision: ('a' if revision == REVISION else 'b') * 64 + with self.assertRaisesRegex(release.ReleaseError, 'Database changes'): + self.deploy(approved_schema='outdated-approval') + self.assert_no_writes() + self.public.calls = 0 + self.assertEqual(self.deploy(approved_schema='a' * 64), DEPLOYMENT) + + def test_failed_coolify_deployment_does_not_count_as_success(self): + self.api.status = 'failed' + with self.assertRaisesRegex(release.ReleaseError, 'status failed'): + self.deploy() + self.assertEqual(self.public.calls, 1) + + def test_old_healthy_public_release_does_not_count_as_success(self): + self.public.version = OLD + with self.assertRaisesRegex(release.ReleaseError, 'release version did not match'): + self.deploy() + + def test_transport_keeps_credentials_out_of_process_arguments_and_does_not_follow_redirects(self): + result = subprocess.CompletedProcess([], 0, stdout='{}\n200', stderr='') + with patch.object(release.subprocess, 'run', return_value=result) as run: + release.Client(release.COOLIFY_URL, 'private-test-token').request('/applications/' + UUID) + args = run.call_args.args[0] + self.assertNotIn('private-test-token', ' '.join(args)) + self.assertNotIn('--location', args) + self.assertEqual(args[1], '--disable') + self.assertIn('Authorization: Bearer private-test-token', run.call_args.kwargs['input']) + + def test_transport_rejects_redirects_without_retrying_a_deploy(self): + result = subprocess.CompletedProcess([], 0, stdout='redirect\n302', stderr='') + with patch.object(release.subprocess, 'run', return_value=result) as run: + with self.assertRaisesRegex(release.ReleaseError, 'HTTP 302'): + release.Client(release.COOLIFY_URL, 'private-test-token').request('/deploy', 'POST', {'uuid': UUID}) + self.assertEqual(run.call_count, 1) + + def test_schema_digest_changes_with_migration_contents(self): + paths = b'prisma/schema.prisma\0prisma/migrations/one/migration.sql\0' + with patch.object(release, 'git', side_effect=[paths, b'SELECT 1;', b'model User {}']): + before = SCHEMA_DIGEST(REVISION) + with patch.object(release, 'git', side_effect=[paths, b'SELECT 2;', b'model User {}']): + after = SCHEMA_DIGEST(REVISION) + self.assertNotEqual(before, after) + + +if __name__ == '__main__': + unittest.main() From dbad987101fdbebb31f945ea22eadc93a571f9dd Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 03:58:16 +0100 Subject: [PATCH 06/16] 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 24b141883ef999e901290f242683927095df18a7 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 03:59:16 +0100 Subject: [PATCH 07/16] ci: retain deployment verification through transient status failures --- .github/workflows/container-image.yml | 4 ++- docs/container-image.md | 7 +++-- scripts/container_deploy.py | 17 +++++++++--- scripts/container_deploy_test.py | 37 ++++++++++++++++++++++++++- 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/.github/workflows/container-image.yml b/.github/workflows/container-image.yml index 2abdc11c7..e5ddfb5bb 100644 --- a/.github/workflows/container-image.yml +++ b/.github/workflows/container-image.yml @@ -28,7 +28,7 @@ jobs: image: if: github.ref == 'refs/heads/master' || github.ref == 'refs/heads/staging' runs-on: ubuntu-latest - timeout-minutes: 45 + timeout-minutes: 70 environment: ${{ github.ref == 'refs/heads/master' && 'container-production' || 'container-staging' }} env: DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/emuready_build @@ -104,6 +104,8 @@ jobs: "${build_args[@]}" --tag "$image:$tag" . test "$(docker image inspect "$image:$tag" --format '{{.Architecture}}')" = amd64 test "$(docker image inspect "$image:$tag" --format '{{.Config.User}}')" = nextjs + docker run --rm --network none --entrypoint node "$image:$tag" \ + -e 'const hook = require("./.next/server/instrumentation.js"); if (typeof hook.register !== "function" || typeof hook.onRequestError !== "function") process.exit(1)' printf 'name=%s\ntag=%s\n' "$image" "$tag" >> "$GITHUB_OUTPUT" - name: Publish image diff --git a/docs/container-image.md b/docs/container-image.md index d818b439f..9f272a35c 100644 --- a/docs/container-image.md +++ b/docs/container-image.md @@ -5,7 +5,7 @@ pass, CI calls **Container image** to build on GitHub runners, publish to privat GHCR, and deploy the immutable digest through Coolify. `staging` deploys to staging; `master` deploys to production. DNS and hosted databases are not changed. -Manual dispatch still builds and optionally publishes an image, with `publish` +Manual dispatch on `master` or `staging` builds and optionally publishes an image, with `publish` defaulting to false. Automatic deployment is called from CI after all checks pass. A branch that has advanced since the workflow started is rejected before deployment. A running release is not cancelled by a newer push. @@ -40,7 +40,10 @@ already embeds its source commit. The public `/api/health` version and readiness must match after Coolify finishes. A queued request alone is not success. The schema and migration files are compared with the current public deployment's -source commit. Any difference blocks deployment. This workflow migrates only its +source commit. Any difference blocks deployment unless the exact schema digest +has been approved after migration verification. The live version must identify +an exact source commit; an unknown baseline must be investigated before release. +This workflow migrates only its disposable runner database; the application does not migrate on startup. For a schema change, verify a fresh backup and obtain explicit approval before diff --git a/scripts/container_deploy.py b/scripts/container_deploy.py index 5011811dc..636e41d1f 100644 --- a/scripts/container_deploy.py +++ b/scripts/container_deploy.py @@ -84,13 +84,13 @@ def preflight(ref, revision, application_uuid, api, public, approved_schema=None raise ReleaseError('Branch advanced; refusing to deploy an outdated release') application = api.request(f'/applications/{application_uuid}') domains = application.get('fqdn', '') - if not isinstance(domains, str) or TARGETS[ref] not in {url.rstrip('/') for url in domains.split(',')}: + if not isinstance(domains, str) or TARGETS[ref] not in {url.strip().rstrip('/') for url in domains.split(',')}: raise ReleaseError('Coolify application domains do not match the selected environment') if (application.get('uuid') != application_uuid or application.get('git_branch') != ref.removeprefix('refs/heads/') or application.get('build_pack') != 'dockerimage' or application.get('docker_registry_image_name') != IMAGE_NAME - or not re.fullmatch('sha256-[a-f0-9]{64}', application.get('docker_registry_image_tag', '')) + or not re.fullmatch('sha256-[a-f0-9]{64}', application.get('docker_registry_image_tag') or '') or application.get('health_check_path') != '/api/health/ready' or not application.get('health_check_enabled') or application.get('pre_deployment_command') @@ -104,7 +104,11 @@ def preflight(ref, revision, application_uuid, api, public, approved_schema=None if deployed.get('status') != 'healthy': raise ReleaseError('The current application is not healthy; investigate before releasing') desired_schema = schema_digest(revision) - if schema_digest(deployed.get('version', '')) != desired_schema and approved_schema != desired_schema: + try: + deployed_schema = schema_digest(deployed.get('version', '')) + except ReleaseError as error: + raise ReleaseError('Cannot verify the deployed source/schema; restore its commit version or fetch its history before releasing') from error + if deployed_schema != desired_schema and approved_schema != desired_schema: raise ReleaseError('Database changes require an approved backup and migration before deployment') return application @@ -129,7 +133,12 @@ def deploy(ref, revision, application_uuid, image, api, public, approved_schema= print(f'Deployment {deployment_uuid}', flush=True) deadline = time.monotonic() + 900 while time.monotonic() < deadline: - deployment = api.request(f'/deployments/{deployment_uuid}') + try: + deployment = api.request(f'/deployments/{deployment_uuid}') + except ReleaseError as error: + print(f'Deployment status unavailable: {error}; retrying', flush=True) + time.sleep(10) + continue status = deployment.get('status') if status == 'finished': break diff --git a/scripts/container_deploy_test.py b/scripts/container_deploy_test.py index cfbec9d8a..0a2210b1e 100644 --- a/scripts/container_deploy_test.py +++ b/scripts/container_deploy_test.py @@ -28,6 +28,7 @@ def __init__(self): } self.variables = [] self.status = 'finished' + self.poll_failures = 0 self.calls = [] def request(self, path, method='GET', payload=None): @@ -37,6 +38,9 @@ def request(self, path, method='GET', payload=None): if path == '/deploy': return {'deployments': [{'resource_uuid': UUID, 'deployment_uuid': DEPLOYMENT}]} if path == '/deployments/' + DEPLOYMENT: + if self.poll_failures: + self.poll_failures -= 1 + raise release.ReleaseError('GET request failed (HTTP 503)') return {'status': self.status} if path.endswith('/envs'): return self.variables @@ -46,13 +50,14 @@ def request(self, path, method='GET', payload=None): class FakePublic: def __init__(self): self.version = REVISION + self.initial_version = OLD self.calls = 0 def request(self, path): if path == '/api/health/ready': return {'status': 'healthy'} self.calls += 1 - return {'status': 'healthy', 'version': OLD if self.calls == 1 else self.version} + return {'status': 'healthy', 'version': self.initial_version if self.calls == 1 else self.version} class DeploymentTests(unittest.TestCase): @@ -108,6 +113,16 @@ def test_rejects_the_wrong_public_domain_without_mutation(self): self.deploy() self.assert_no_writes() + def test_accepts_whitespace_between_configured_domains(self): + self.api.application['fqdn'] = 'https://other.example, https://staging.emuready.com/ ' + self.assertEqual(self.deploy(), DEPLOYMENT) + + def test_rejects_a_null_image_selection_without_mutation(self): + self.api.application['docker_registry_image_tag'] = None + with self.assertRaisesRegex(release.ReleaseError, 'target'): + self.deploy() + self.assert_no_writes() + def test_master_uses_the_production_application(self): ref = 'refs/heads/master' self.branch.return_value = f'{REVISION}\t{ref}\n'.encode() @@ -134,6 +149,26 @@ def test_failed_coolify_deployment_does_not_count_as_success(self): self.deploy() self.assertEqual(self.public.calls, 1) + def test_retries_a_transient_status_read_without_queuing_another_deployment(self): + self.api.poll_failures = 1 + self.assertEqual(self.deploy(), DEPLOYMENT) + self.assertEqual(sum(path == '/deploy' for path, _, _ in self.api.calls), 1) + self.assertEqual(sum(path == '/deployments/' + DEPLOYMENT for path, _, _ in self.api.calls), 2) + + def test_status_read_failures_remain_bounded_by_the_deadline(self): + self.api.poll_failures = 1 + with patch.object(release.time, 'monotonic', side_effect=[0, 0, 901]): + with self.assertRaisesRegex(release.ReleaseError, 'Timed out waiting'): + self.deploy() + self.assertEqual(sum(path == '/deploy' for path, _, _ in self.api.calls), 1) + + def test_schema_approval_does_not_bypass_an_unknown_live_source_commit(self): + self.public.initial_version = '0.0.0' + self.schema.side_effect = lambda revision: 'a' * 64 if revision == REVISION else SCHEMA_DIGEST(revision) + with self.assertRaisesRegex(release.ReleaseError, 'Cannot verify the deployed source/schema'): + self.deploy(approved_schema='a' * 64) + self.assert_no_writes() + def test_old_healthy_public_release_does_not_count_as_success(self): self.public.version = OLD with self.assertRaisesRegex(release.ReleaseError, 'release version did not match'): From 77f3b8a16bb877c58d702fbc263f02a559a9c3f7 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 05:34:54 +0100 Subject: [PATCH 08/16] 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 09/16] 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 34cee27ef7d80337fbc2680c7feddf4e680f786f Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 05:35:42 +0100 Subject: [PATCH 10/16] ci: recheck release head and retain health verification failures --- .github/workflows/ci.yml | 1 + docs/container-image.md | 10 +++++++++- scripts/container_deploy.py | 30 +++++++++++++++++++++++------- scripts/container_deploy_test.py | 30 ++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0d69fbcf7..5adac02dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -187,6 +187,7 @@ jobs: browser-tests: name: Browser tests + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository uses: ./.github/workflows/e2e-tests.yml secrets: inherit diff --git a/docs/container-image.md b/docs/container-image.md index 9f272a35c..89c0e2e4c 100644 --- a/docs/container-image.md +++ b/docs/container-image.md @@ -8,7 +8,15 @@ staging; `master` deploys to production. DNS and hosted databases are not change Manual dispatch on `master` or `staging` builds and optionally publishes an image, with `publish` defaulting to false. Automatic deployment is called from CI after all checks pass. A branch that has advanced since the workflow started is rejected before -deployment. A running release is not cancelled by a newer push. +deployment, including a second head check immediately before selecting the image. +GitHub and Coolify do not share an atomic revision lock: a merge after that check +can briefly leave the earlier revision running until the next release finishes. +A running release is not cancelled by a newer push. + +Authenticated browser CI runs for repository branches and their pull requests. +Fork pull requests skip that job because GitHub does not provide repository +secrets to them; browser tests still run after an accepted merge into `staging` +or `master`, before publication or deployment. Create a GitHub Actions environment **variable** named `CONTAINER_PUBLIC_BUILD_ENV` containing a JSON object in each environment below. The workflow reads it through diff --git a/scripts/container_deploy.py b/scripts/container_deploy.py index 636e41d1f..db3d9e5c0 100644 --- a/scripts/container_deploy.py +++ b/scripts/container_deploy.py @@ -74,14 +74,18 @@ def schema_digest(revision): return digest.hexdigest() +def verify_branch_head(ref, revision): + remote = git('ls-remote', '--exit-code', 'origin', ref).decode().split() + if len(remote) != 2 or remote[0] != revision or remote[1] != ref: + raise ReleaseError('Branch advanced; refusing to deploy an outdated release') + + def preflight(ref, revision, application_uuid, api, public, approved_schema=None): if ref not in TARGETS or not re.fullmatch('[a-f0-9]{40}', revision): raise ReleaseError('Releases require an exact staging or master commit') if not re.fullmatch('[a-zA-Z0-9]{10,64}', application_uuid): raise ReleaseError('Set the target environment application UUID') - remote = git('ls-remote', '--exit-code', 'origin', ref).decode().split() - if len(remote) != 2 or remote[0] != revision or remote[1] != ref: - raise ReleaseError('Branch advanced; refusing to deploy an outdated release') + verify_branch_head(ref, revision) application = api.request(f'/applications/{application_uuid}') domains = application.get('fqdn', '') if not isinstance(domains, str) or TARGETS[ref] not in {url.strip().rstrip('/') for url in domains.split(',')}: @@ -119,6 +123,7 @@ def deploy(ref, revision, application_uuid, image, api, public, approved_schema= application = preflight(ref, revision, application_uuid, api, public, approved_schema) previous_image = f'{IMAGE_NAME}@sha256:{application["docker_registry_image_tag"].removeprefix("sha256-")}' print(f'Previous image: {previous_image}', flush=True) + verify_branch_head(ref, revision) # Coolify restricts the deploy API's docker_tag override to preview deployments. api.request(f'/applications/{application_uuid}', 'PATCH', { 'docker_registry_image_tag': image.split('@')[1].replace('sha256:', 'sha256-'), @@ -147,17 +152,28 @@ def deploy(ref, revision, application_uuid, image, api, public, approved_schema= time.sleep(10) else: raise ReleaseError('Timed out waiting for Coolify; inspect the deployment before retrying') + last_error = 'No public health response received' for attempt in range(6): try: health = public.request('/api/health') ready = public.request('/api/health/ready') - if health.get('version') == revision and health.get('status') == 'healthy' and ready.get('status') == 'healthy': + if not isinstance(health, dict) or not isinstance(ready, dict): + raise ReleaseError('Expected public health responses to be JSON objects') + failures = [] + if health.get('version') != revision: + failures.append('release version differs from the expected commit') + if health.get('status') != 'healthy': + failures.append('liveness is not healthy') + if ready.get('status') != 'healthy': + failures.append('readiness is not healthy') + if not failures: return deployment_uuid - except ReleaseError: - pass + last_error = '; '.join(failures) + except ReleaseError as error: + last_error = str(error) if attempt < 5: time.sleep(10) - raise ReleaseError('Deployment finished, but public readiness and release version did not match') + raise ReleaseError(f'Deployment finished, but public readiness and release version did not match: {last_error}') def main(): diff --git a/scripts/container_deploy_test.py b/scripts/container_deploy_test.py index 0a2210b1e..e37fdca75 100644 --- a/scripts/container_deploy_test.py +++ b/scripts/container_deploy_test.py @@ -95,6 +95,13 @@ def test_rejects_a_stale_build_without_mutation(self): self.deploy() self.assertEqual(self.api.calls, []) + def test_rechecks_branch_after_preflight_before_any_mutation(self): + self.branch.side_effect = [f'{REVISION}\t{REF}\n'.encode(), f'{OLD}\t{REF}\n'.encode()] + with self.assertRaisesRegex(release.ReleaseError, 'Branch advanced'): + self.deploy() + self.assert_no_writes() + self.assertEqual(self.branch.call_count, 2) + def test_rejects_wrong_environment_without_mutation(self): self.api.application['git_branch'] = 'master' with self.assertRaisesRegex(release.ReleaseError, 'target'): @@ -174,6 +181,29 @@ def test_old_healthy_public_release_does_not_count_as_success(self): with self.assertRaisesRegex(release.ReleaseError, 'release version did not match'): self.deploy() + def test_retries_non_object_health_json_without_repeating_the_deployment(self): + healthy = {'status': 'healthy'} + responses = [{**healthy, 'version': OLD}, [], healthy, {**healthy, 'version': REVISION}, healthy] + with patch.object(self.public, 'request', side_effect=responses): + self.assertEqual(self.deploy(), DEPLOYMENT) + self.assertEqual(sum(path == '/deploy' for path, _, _ in self.api.calls), 1) + + def test_includes_the_last_transport_failure_in_the_readiness_error(self): + responses = [{'status': 'healthy', 'version': OLD}] + responses += [release.ReleaseError('GET request failed (HTTP 503)') for _ in range(6)] + with patch.object(self.public, 'request', side_effect=responses): + with self.assertRaisesRegex(release.ReleaseError, 'HTTP 503'): + self.deploy() + + def test_includes_the_failed_health_gate_without_echoing_response_data(self): + responses = [{'status': 'healthy', 'version': OLD}] + responses += [{'status': 'healthy', 'version': REVISION}, {'status': 'private-upstream-detail'}] * 6 + with patch.object(self.public, 'request', side_effect=responses): + with self.assertRaises(release.ReleaseError) as result: + self.deploy() + self.assertIn('readiness is not healthy', str(result.exception)) + self.assertNotIn('private-upstream-detail', str(result.exception)) + def test_transport_keeps_credentials_out_of_process_arguments_and_does_not_follow_redirects(self): result = subprocess.CompletedProcess([], 0, stdout='{}\n200', stderr='') with patch.object(release.subprocess, 'run', return_value=result) as run: From b338e804e215157f1d4dccefc4f1d8d51945fe0d Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 05:52:33 +0100 Subject: [PATCH 11/16] 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 12/16] 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 6305fcee5ab370ebfe14cfe0fa1c4f0467ebd73c Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 06:00:09 +0100 Subject: [PATCH 13/16] ci: disable persisted checkout credentials before running dependencies --- .github/workflows/ci.yml | 6 ++++++ .github/workflows/e2e-tests.yml | 2 ++ 2 files changed, 8 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5adac02dd..3b512afdd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,6 +56,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v6 + with: + persist-credentials: false - name: Test container build configuration run: python3 -m unittest discover -s scripts -p container_build_config_test.py @@ -110,6 +112,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v6 + with: + persist-credentials: false - name: Setup pnpm uses: pnpm/action-setup@v6 @@ -155,6 +159,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v6 + with: + persist-credentials: false - name: Setup pnpm uses: pnpm/action-setup@v6 diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index a06607af4..f1833a7a7 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -52,6 +52,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v6 + with: + persist-credentials: false - name: Setup pnpm uses: pnpm/action-setup@v6 From 56b01c0a464d86082ac1ab2d454dbb40bb04ad44 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 07:59:42 +0100 Subject: [PATCH 14/16] 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() +} From a0efe4bf9f1e08815c0651c9686e7077580ce785 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 08:00:17 +0100 Subject: [PATCH 15/16] docs: clarify release ordering when newer pushes arrive --- docs/container-image.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/container-image.md b/docs/container-image.md index 89c0e2e4c..c57c6693a 100644 --- a/docs/container-image.md +++ b/docs/container-image.md @@ -7,11 +7,13 @@ staging; `master` deploys to production. DNS and hosted databases are not change Manual dispatch on `master` or `staging` builds and optionally publishes an image, with `publish` defaulting to false. Automatic deployment is called from CI after all checks pass. -A branch that has advanced since the workflow started is rejected before -deployment, including a second head check immediately before selecting the image. +The branch head is checked before building and immediately before selecting the +image. If it has advanced at either check, the release stops. GitHub and Coolify do not share an atomic revision lock: a merge after that check can briefly leave the earlier revision running until the next release finishes. -A running release is not cancelled by a newer push. +A running release is not cancelled by a newer push, so Coolify verification can +finish after image selection and deployment have started. The next release waits +for it and must pass its own CI and deployment checks. Authenticated browser CI runs for repository branches and their pull requests. Fork pull requests skip that job because GitHub does not provide repository From 121d0e4301e7398eb0f6785e0f6a87d321944c86 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Sat, 31 Oct 2026 18:12:40 +0100 Subject: [PATCH 16/16] docs: scope branch checks to automatic deployments --- docs/container-image.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/container-image.md b/docs/container-image.md index c57c6693a..2216aa8f7 100644 --- a/docs/container-image.md +++ b/docs/container-image.md @@ -7,8 +7,8 @@ staging; `master` deploys to production. DNS and hosted databases are not change Manual dispatch on `master` or `staging` builds and optionally publishes an image, with `publish` defaulting to false. Automatic deployment is called from CI after all checks pass. -The branch head is checked before building and immediately before selecting the -image. If it has advanced at either check, the release stops. +For automatic deployments, the branch head is checked before building and +immediately before selecting the image. If it has advanced at either check, the release stops. GitHub and Coolify do not share an atomic revision lock: a merge after that check can briefly leave the earlier revision running until the next release finishes. A running release is not cancelled by a newer push, so Coolify verification can