diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93bd09782..3b512afdd 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" @@ -56,10 +56,15 @@ 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 + - 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 +79,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 @@ -107,6 +112,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v6 + with: + persist-credentials: false - name: Setup pnpm uses: pnpm/action-setup@v6 @@ -122,7 +129,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 @@ -152,6 +159,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v6 + with: + persist-credentials: false - name: Setup pnpm uses: pnpm/action-setup@v6 @@ -170,10 +179,33 @@ 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 + + - 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)' + + 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 + + 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..e5ddfb5bb 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: @@ -20,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 @@ -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 @@ -84,9 +104,12 @@ 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 + id: publish if: inputs.publish env: GHCR_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -98,4 +121,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 9d170853b..f1833a7a7 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -1,35 +1,15 @@ 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-latest + runs-on: ubuntu-24.04 timeout-minutes: 30 services: @@ -72,6 +52,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v6 + with: + persist-credentials: false - name: Setup pnpm uses: pnpm/action-setup@v6 @@ -87,13 +69,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..2216aa8f7 100644 --- a/docs/container-image.md +++ b/docs/container-image.md @@ -1,7 +1,24 @@ # 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 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. +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 +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 +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 @@ -19,14 +36,39 @@ 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 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 +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 +80,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/instrumentation.ts b/instrumentation.ts deleted file mode 100644 index 0827d94b9..000000000 --- a/instrumentation.ts +++ /dev/null @@ -1,15 +0,0 @@ -/** - * Next.js Instrumentation - * This file runs once when the Next.js server starts - * for one-time initialization of services - */ - -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() -} diff --git a/next.config.ts b/next.config.ts index 9bee1bcb4..edaf7f3e8 100644 --- a/next.config.ts +++ b/next.config.ts @@ -324,32 +324,17 @@ 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 + release: { name: nextBuildId || undefined }, 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/scripts/container_deploy.py b/scripts/container_deploy.py new file mode 100644 index 000000000..db3d9e5c0 --- /dev/null +++ b/scripts/container_deploy.py @@ -0,0 +1,214 @@ +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 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') + 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(',')}: + 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 '') + 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) + 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 + + +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) + 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-'), + '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: + 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 + 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') + 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 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 + last_error = '; '.join(failures) + except ReleaseError as error: + last_error = str(error) + if attempt < 5: + time.sleep(10) + raise ReleaseError(f'Deployment finished, but public readiness and release version did not match: {last_error}') + + +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..e37fdca75 --- /dev/null +++ b/scripts/container_deploy_test.py @@ -0,0 +1,234 @@ +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.poll_failures = 0 + 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: + 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 + return self.application + + +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': self.initial_version 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_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'): + 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_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() + 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_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'): + 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: + 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() 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..36cd652f1 100644 --- a/sentry.server.config.ts +++ b/sentry.server.config.ts @@ -1,21 +1,14 @@ -// 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' +import { env } from '@/lib/env' -// 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: env.APP_ENV, + ...(process.env.APP_VERSION ? { 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..c7acfeabc --- /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 index af8537305..d50c1e938 100644 --- a/src/instrumentation.ts +++ b/src/instrumentation.ts @@ -1,12 +1,13 @@ import * as Sentry from '@sentry/nextjs' +import { PHASE_PRODUCTION_BUILD } from 'next/constants' export async function register() { + if (process.env.NEXT_PHASE === PHASE_PRODUCTION_BUILD) return + if (process.env.NEXT_RUNTIME === 'nodejs') { await import('../sentry.server.config') - } - - if (process.env.NEXT_RUNTIME === 'edge') { - await import('../sentry.edge.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..863b17ba6 100644 --- a/src/lib/app-error-cause.test.ts +++ b/src/lib/app-error-cause.test.ts @@ -67,4 +67,51 @@ describe('getSerializableAppError', () => { expect(getSerializableAppError('nope')).toBeNull() 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) => { + 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/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/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-build-config.test.ts b/src/sentry-build-config.test.ts new file mode 100644 index 000000000..6e210fe6e --- /dev/null +++ b/src/sentry-build-config.test.ts @@ -0,0 +1,28 @@ +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) +}) + +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/src/sentry-config.test.ts b/src/sentry-config.test.ts index 9aebfa0f5..ad18a3fce 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,21 @@ afterEach(() => { vi.clearAllMocks() }) -describe('Sentry server and edge config', () => { +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', @@ -35,18 +48,31 @@ 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', }), ) }) + + 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 389f9b96a..0acaa132f 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) } }), @@ -65,8 +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') + 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/api/trpc-errors.test.ts b/src/server/api/trpc-errors.test.ts new file mode 100644 index 000000000..221317a82 --- /dev/null +++ b/src/server/api/trpc-errors.test.ts @@ -0,0 +1,102 @@ +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() + 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) + + expect(response.status).toBe(500) + expect(mocks.error).toHaveBeenCalledExactlyOnceWith('[tRPC] Server error', error, { + procedure: 'probe', + code: 'INTERNAL_SERVER_ERROR', + }) + }) + + 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], + ] 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..9cea0a4c8 --- /dev/null +++ b/src/server/api/trpc-errors.ts @@ -0,0 +1,17 @@ +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) { + 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 ?? '', + code: error.code, + }) +} 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 new file mode 100644 index 000000000..4919f2ece --- /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 an initializer rejection 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 cbb1f90ce..2eae5c125 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 { type PrismaClient } from '@orm/client' +import { getSerializableAppError } from '@/lib/app-error-cause' +import { Prisma, 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,53 @@ 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' }, + ) + } + }) + + 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 ad4836316..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 @@ -91,8 +89,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) } } diff --git a/src/server/utils/switchGameSearch.test.ts b/src/server/utils/switchGameSearch.test.ts new file mode 100644 index 000000000..4862f28c4 --- /dev/null +++ b/src/server/utils/switchGameSearch.test.ts @@ -0,0 +1,136 @@ +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.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') + + 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..84979be85 100644 --- a/src/server/utils/switchGameSearch.ts +++ b/src/server/utils/switchGameSearch.ts @@ -45,10 +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) { @@ -76,16 +80,20 @@ 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 - } + if (switchGamesDataPromise) return switchGamesDataPromise + const generation = cacheGeneration + const load: ReturnType = fetchSwitchGamesData() + .then((freshData) => { + if (generation === cacheGeneration) { + switchGamesDataCache.set(cacheKey, { data: freshData, createdAt: new Date() }) + } + return freshData + }) + .finally(() => { + if (switchGamesDataPromise === load) switchGamesDataPromise = null + }) + switchGamesDataPromise = load + return load } async function getFuseInstance(): Promise> { @@ -94,10 +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 } @@ -188,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 d0632adf6..0dbdbfb44 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' @@ -85,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 () => { @@ -109,6 +211,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..dff03e281 100644 --- a/src/server/utils/threeDsGameSearch.ts +++ b/src/server/utils/threeDsGameSearch.ts @@ -77,6 +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, @@ -136,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, }), ]) @@ -207,16 +213,20 @@ 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 - } + if (threeDsGamesDataPromise) return threeDsGamesDataPromise + const generation = cacheGeneration + const load: ReturnType = fetchThreeDsGamesData() + .then((fresh) => { + if (generation === cacheGeneration) { + threeDsGamesDataCache.set(cacheKey, { data: fresh, createdAt: new Date() }) + } + return fresh + }) + .finally(() => { + if (threeDsGamesDataPromise === load) threeDsGamesDataPromise = null + }) + threeDsGamesDataPromise = load + return load } async function getFuseInstance(): Promise> { @@ -224,9 +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 } @@ -343,8 +356,7 @@ export async function getBestThreeDsTitleIdMatch(gameName: string): Promise { - threeDsGamesDataCache.clear() - threeDsGamesFuseCache.clear() + clearThreeDsCaches() await getThreeDsGamesData() await getFuseInstance() } @@ -381,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() +} 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) 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..6726bbb81 100644 --- a/tests/error-handling.spec.ts +++ b/tests/error-handling.spec.ts @@ -16,12 +16,19 @@ 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')) + const failedGamesRequest = page.waitForEvent( + 'requestfailed', + (request) => request.url().includes('/api/trpc/') && request.url().includes('games.get'), + ) await page.goto('/games', { waitUntil: 'domcontentloaded' }) + await failedGamesRequest - 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 }) => {