From 6089adbaa8700cbff4ba38a36e1f24a3ba3e491e Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Wed, 6 May 2026 14:17:41 +0000 Subject: [PATCH 01/13] test/e2e/s3: add tests for S3 server 5xx Ref #1862 --- test/e2e/s3/test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index 03f86bd23..7eee35bfb 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -304,6 +304,24 @@ describe('s3 support', () => { }); }); + describe('with minio 5xx', () => { + before(async () => { + // TODO start a fake s3 server which replies 500 to everything + }); + + after(async () => { + // TODO stop the fake s3 server + }); + + it('should handle upload failure gracefully', async () => { + throw new Error('implement me'); + }); + + it('should handle download failure gracefully', async () => { + throw new Error('implement me'); + }); + }); + // Guard against a Promise resolving when it was expected to reject. This has // specifically been seen when upload-pending returns immediately, but later // test code is expecting it to spend time uploading. In those cases, this From 0ccd282e6a9e04163cd90d8b17777ecda082d8dc Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 10:59:22 +0000 Subject: [PATCH 02/13] implement bad server --- test/e2e/s3/test.js | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index 7eee35bfb..b8215cc80 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -15,6 +15,8 @@ const TIMEOUT = 240_000; // ms const { exec, execSync } = require('node:child_process'); const fs = require('node:fs'); const { randomBytes } = require('node:crypto'); + +const express = require('express'); const _ = require('lodash'); const should = require('should'); @@ -305,12 +307,23 @@ describe('s3 support', () => { }); describe('with minio 5xx', () => { - before(async () => { - // TODO start a fake s3 server which replies 500 to everything + let server; + + before(() => { + const app = express(); + + app.all('/*', (req, res) => res.sendStatus(500)); + + return new Promise((resolve, reject) => { + server = app.listen(9000, err => { + if(err) return reject(err); + resolve(); + }); + }); }); - after(async () => { - // TODO stop the fake s3 server + after(() => { + if(server) return new Promise(resolve => server.close(resolve)); }); it('should handle upload failure gracefully', async () => { From 2f72b1bb5a78de9a07f9728698175e87c8fabd1c Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:00:13 +0000 Subject: [PATCH 03/13] implement 1 test --- test/e2e/s3/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index b8215cc80..cad7e5b0a 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -331,7 +331,7 @@ describe('s3 support', () => { }); it('should handle download failure gracefully', async () => { - throw new Error('implement me'); + await expectRejectionFrom(forSacrifice(cli('upload-pending')), /Error: TODO/); }); }); From d04e7608cd8bc0062da5548cacfd46184e0e5492 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:12:16 +0000 Subject: [PATCH 04/13] dig into err a bit --- lib/external/s3.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/external/s3.js b/lib/external/s3.js index ed4697007..a682138ce 100644 --- a/lib/external/s3.js +++ b/lib/external/s3.js @@ -101,6 +101,13 @@ const init = (config) => { const isErrUpstream = err => err.name === 'S3Error' && err.code === 'InternalError'; const wrappedOrUnhandled = (err, operation, blobOrBlobs) => { const details = { amzRequestId: err.requestid, operation }; + + console.log('s3.wrappedOrUnhandled()', 'err:', err); + console.log('s3.wrappedOrUnhandled()', 'err.name:', err.name); + console.log('s3.wrappedOrUnhandled()', 'err.code:', err.code); + console.log('s3.wrappedOrUnhandled()', 'err.message:', err.message); + console.log('s3.wrappedOrUnhandled()', 'err.props:', Object.keys(err)); + if (isErrUpstream(err)) { if (Array.isArray(blobOrBlobs)) details.blobIds = blobOrBlobs.map(blob => blob.id); else details.blobId = blobOrBlobs.id; From 7ecdbf8696da462fdf465eef33664263985d4d3a Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:14:53 +0000 Subject: [PATCH 05/13] remove irrelevant CI jobs --- .github/workflows/db-migrations.yml | 51 --------------------- .github/workflows/oidc-e2e.yml | 49 --------------------- .github/workflows/oidc-integration.yml | 39 ---------------- .github/workflows/soak-test.yml | 41 ----------------- .github/workflows/standard-e2e.yml | 61 -------------------------- 5 files changed, 241 deletions(-) delete mode 100644 .github/workflows/db-migrations.yml delete mode 100644 .github/workflows/oidc-e2e.yml delete mode 100644 .github/workflows/oidc-integration.yml delete mode 100644 .github/workflows/soak-test.yml delete mode 100644 .github/workflows/standard-e2e.yml diff --git a/.github/workflows/db-migrations.yml b/.github/workflows/db-migrations.yml deleted file mode 100644 index 78f8b0361..000000000 --- a/.github/workflows/db-migrations.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: Database Migrations - -on: - pull_request: - paths: - - .github/workflows/db-migrations.yml - - lib/bin/create-docker-databases.js - - lib/model/migrations/** - - test/db-migrations/** - - package.json - - package-lock.json - - Makefile - push: - paths: - - .github/workflows/db-migrations.yml - - lib/bin/create-docker-databases.js - - lib/model/migrations/** - - test/db-migrations/** - - package.json - - package-lock.json - - Makefile - -jobs: - db-migration-tests: - timeout-minutes: 2 - # TODO should we use the same container as circle & central? - runs-on: ubuntu-latest - services: - # see: https://docs.github.com/en/enterprise-server@3.5/actions/using-containerized-services/creating-postgresql-service-containers - postgres: - image: postgres:14.20 - env: - POSTGRES_PASSWORD: odktest - ports: - - 5432:5432 - # Set health checks to wait until postgres has started - options: >- - --health-cmd pg_isready - --health-interval 1s - --health-timeout 5s - --health-retries 50 - steps: - - uses: actions/checkout@v6 - - name: Set node version - uses: actions/setup-node@v6 - with: - node-version-file: package.json - cache: 'npm' - - run: npm ci - - run: node lib/bin/create-docker-databases.js - - run: make test-db-migrations diff --git a/.github/workflows/oidc-e2e.yml b/.github/workflows/oidc-e2e.yml deleted file mode 100644 index 5b5851459..000000000 --- a/.github/workflows/oidc-e2e.yml +++ /dev/null @@ -1,49 +0,0 @@ -name: OIDC e2e tests - -on: - push: - pull_request: - -env: - DEBUG: pw:api - ODK_PLAYWRIGHT_BROWSERS: chromium,firefox,webkit - -jobs: - oidc-e2e-test: - timeout-minutes: 6 - # TODO should we use the same container as circle & central? - runs-on: ubuntu-latest - services: - # see: https://docs.github.com/en/enterprise-server@3.5/actions/using-containerized-services/creating-postgresql-service-containers - postgres: - image: postgres:14.20 - env: - POSTGRES_PASSWORD: odktest - ports: - - 5432:5432 - # Set health checks to wait until postgres has started - options: >- - --health-cmd pg_isready - --health-interval 1s - --health-timeout 5s - --health-retries 50 - steps: - - # This one weird trick speeds up every build! - # see: https://github.com/getodk/central-backend/pull/1642 - # see: https://github.com/actions/runner/issues/4030 - - run: sudo apt-get remove --purge man-db - - - uses: actions/checkout@v6 - - name: Set node version - uses: actions/setup-node@v6 - with: - node-version-file: package.json - cache: 'npm' - - run: make test-oidc-e2e - - name: Archive Playwright Test Report - if: failure() - uses: actions/upload-artifact@v4 - with: - name: Playwright Test Report - path: test/e2e/oidc/playwright-tests/results/html-report/ diff --git a/.github/workflows/oidc-integration.yml b/.github/workflows/oidc-integration.yml deleted file mode 100644 index b38e1a7e0..000000000 --- a/.github/workflows/oidc-integration.yml +++ /dev/null @@ -1,39 +0,0 @@ -name: OIDC integration tests - -on: - push: - pull_request: - -jobs: - oidc-integration-test: - timeout-minutes: 6 - # TODO should we use the same container as circle & central? - runs-on: ubuntu-latest - services: - # see: https://docs.github.com/en/enterprise-server@3.5/actions/using-containerized-services/creating-postgresql-service-containers - postgres: - image: postgres:14.20 - env: - POSTGRES_PASSWORD: odktest - ports: - - 5432:5432 - # Set health checks to wait until postgres has started - options: >- - --health-cmd pg_isready - --health-interval 1s - --health-timeout 5s - --health-retries 50 - steps: - - uses: actions/checkout@v6 - - name: Set node version - uses: actions/setup-node@v6 - with: - node-version-file: package.json - cache: 'npm' - - run: npm ci - - run: FAKE_OIDC_ROOT_URL=http://localhost:9898 make fake-oidc-server-ci > fake-oidc-server.log & - - run: node lib/bin/create-docker-databases.js - - run: make test-oidc-integration - - name: Fake OIDC Server Logs - if: always() - run: "! [[ -f ./fake-oidc-server.log ]] || cat ./fake-oidc-server.log" diff --git a/.github/workflows/soak-test.yml b/.github/workflows/soak-test.yml deleted file mode 100644 index 2215a73d7..000000000 --- a/.github/workflows/soak-test.yml +++ /dev/null @@ -1,41 +0,0 @@ -name: Soak Test - -on: - push: - branches: master - -jobs: - soak-test: - timeout-minutes: 15 - # TODO should we use the same container as circle & central? - runs-on: ubuntu-latest - services: - # see: https://docs.github.com/en/enterprise-server@3.5/actions/using-containerized-services/creating-postgresql-service-containers - postgres: - image: postgres:14.20 - env: - POSTGRES_PASSWORD: odktest - ports: - - 5432:5432 - # Set health checks to wait until postgres has started - options: >- - --health-cmd pg_isready - --health-interval 1s - --health-timeout 5s - --health-retries 50 - steps: - - uses: actions/checkout@v6 - - run: ./test/e2e/soak/ci/prepare-postgres.sh - - name: Set node version - uses: actions/setup-node@v6 - with: - node-version-file: package.json - cache: 'npm' - - run: npm ci - - run: node lib/bin/create-docker-databases.js - - name: Soak Test - timeout-minutes: 10 - run: ./test/e2e/soak/ci/run-tests.sh - - name: Backend Logs - if: always() - run: "! [[ -f ./backend.log ]] || cat ./backend.log" diff --git a/.github/workflows/standard-e2e.yml b/.github/workflows/standard-e2e.yml deleted file mode 100644 index 5109f94b2..000000000 --- a/.github/workflows/standard-e2e.yml +++ /dev/null @@ -1,61 +0,0 @@ -name: Standard E2E Tests - -on: - push: - pull_request: - -env: - LOG_LEVEL: DEBUG - -jobs: - standard-e2e: - timeout-minutes: 15 - # TODO should we use the same container as circle & central? - runs-on: ubuntu-latest - services: - # see: https://docs.github.com/en/enterprise-server@3.5/actions/using-containerized-services/creating-postgresql-service-containers - postgres: - image: postgres:14.20 - env: - POSTGRES_PASSWORD: odktest - ports: - - 5432:5432 - # Set health checks to wait until postgres has started - options: >- - --health-cmd pg_isready - --health-interval 1s - --health-timeout 5s - --health-retries 50 - steps: - - uses: actions/checkout@v6 - - name: Set node version - uses: actions/setup-node@v6 - with: - node-version-file: package.json - cache: 'npm' - - run: npm ci - - run: node lib/bin/create-docker-databases.js - - - name: E2E Test - timeout-minutes: 10 - run: ./test/e2e/standard/run-tests.sh - - - uses: ./.github/actions/install-postgres-client - with: - postgres-version: 14 - - name: Grant postgres superuser role - # As per #1368, this role is required to create the pgrowlocks extension - run: psql -c 'ALTER ROLE jubilant SUPERUSER' postgresql://postgres:odktest@localhost/postgres - - name: Backup/restore tests - timeout-minutes: 10 - run: ./test/e2e/standard/backup-restore.sh - - name: Backup test - test detection of aborted backup process - sabotage DB - run: psql -c 'ALTER ROLE jubilant NOSUPERUSER; REVOKE SELECT ON TABLE knex_migrations FROM jubilant;' postgresql://postgres:odktest@localhost/jubilant - - name: Backup test - test detection of aborted backup process - timeout-minutes: 2 - run: ./test/e2e/standard/backup-abortstream.sh - - name: Backup test - test detection of aborted backup process - unsabotage DB - run: psql -c 'GRANT SELECT ON TABLE knex_migrations TO jubilant' postgresql://postgres:odktest@localhost/jubilant - - name: Backend Logs - if: always() - run: "! [[ -f ./server.log ]] || cat ./server.log" From bd9da155b976b3b00e26232480c6ef990731786e Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:15:30 +0000 Subject: [PATCH 06/13] dissable other e2e test --- test/e2e/s3/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index cad7e5b0a..db3081452 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -306,7 +306,7 @@ describe('s3 support', () => { }); }); - describe('with minio 5xx', () => { + describe.only('with minio 5xx', () => { let server; before(() => { From 8a62b1604c209780930f0f666ba5227c76db6586 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:17:36 +0000 Subject: [PATCH 07/13] ci/s3-e2e: show server logs at end of run --- .github/workflows/s3-e2e.yml | 3 +++ test/e2e/s3/run-tests.sh | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/s3-e2e.yml b/.github/workflows/s3-e2e.yml index 0fad3c5e5..715f6d660 100644 --- a/.github/workflows/s3-e2e.yml +++ b/.github/workflows/s3-e2e.yml @@ -37,3 +37,6 @@ jobs: - name: E2E Test timeout-minutes: 10 run: ./test/e2e/s3/run-tests.sh + - name: Backend Logs + if: always() + run: "! [[ -f ./server.log ]] || cat ./server.log" diff --git a/test/e2e/s3/run-tests.sh b/test/e2e/s3/run-tests.sh index dfee1f284..2bdc471b1 100755 --- a/test/e2e/s3/run-tests.sh +++ b/test/e2e/s3/run-tests.sh @@ -56,7 +56,7 @@ log "Waiting for fake-s3-server to start..." wait-for-it localhost:9000 --strict --timeout=10 -- echo '[test/e2e/s3/run-tests] fake-s3-server is UP!' NODE_CONFIG_ENV=s3-dev node lib/bin/s3-create-bucket.js -NODE_CONFIG_ENV=s3-dev make run & +NODE_CONFIG_ENV=s3-dev make run | tee server.log & serverPid=$! log 'Waiting for backend to start...' From 202c2e55dfc1c7172853bc0e1c1e92c63250cccc Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:21:55 +0000 Subject: [PATCH 08/13] minioTerminated --- test/e2e/s3/test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index db3081452..f50320e95 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -310,6 +310,8 @@ describe('s3 support', () => { let server; before(() => { + minioTerminated(); + const app = express(); app.all('/*', (req, res) => res.sendStatus(500)); From 55a02c1b5be78bdfb05ce8ceb49a5abb4b178cc1 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:33:22 +0000 Subject: [PATCH 09/13] add logging --- test/e2e/s3/test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index f50320e95..1c4cd3e50 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -36,6 +36,8 @@ describe('s3 support', () => { const minioTerminated = () => { if(_minioTerminated) return; + log('Terminating minio...'); + // It should be possible to use docker more precisely here, e.g. // docker stop $(docker ps --quiet --filter "ancestor=minio/minio") // However, the ancestor filter requries specifying the exact tag used. From ad78aaae0aaacea8acab76ce256529d2b29792bb Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:34:00 +0000 Subject: [PATCH 10/13] remove .only --- test/e2e/s3/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index 1c4cd3e50..88e71cd3d 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -308,7 +308,7 @@ describe('s3 support', () => { }); }); - describe.only('with minio 5xx', () => { + describe('with minio 5xx', () => { let server; before(() => { From d0d3c7914afff393ef371d43f525cea6f0b286e2 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:35:58 +0000 Subject: [PATCH 11/13] more logging --- test/e2e/s3/test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index 88e71cd3d..8aba87792 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -38,11 +38,14 @@ describe('s3 support', () => { log('Terminating minio...'); + console.log(execSync('docker ps')); + // It should be possible to use docker more precisely here, e.g. // docker stop $(docker ps --quiet --filter "ancestor=minio/minio") // However, the ancestor filter requries specifying the exact tag used. // See: https://docs.docker.com/reference/cli/docker/container/ls/#ancestor execSync(`docker ps | awk '/minio/ { print $1 }' | xargs docker kill`); + console.log(execSync('docker ps')); _minioTerminated = true; }; From 9736100a611b770e7285c748f8de5c73a584212a Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Thu, 7 May 2026 11:48:03 +0000 Subject: [PATCH 12/13] more deubg --- test/e2e/s3/test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index 8aba87792..d99e438af 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -458,6 +458,7 @@ function cli(cmd) { const promise = new Promise((resolve, reject) => { const child = exec(cmd, { env, cwd:'../../..' }, (err, stdout, stderr) => { + console.log({ stdout, stderr }); if (err) { err.stdout = stdout; // eslint-disable-line no-param-reassign err.stderr = stderr; // eslint-disable-line no-param-reassign From e49559514f1886b8df167b8ec2d56d7025b835d1 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Tue, 19 May 2026 13:15:30 +0000 Subject: [PATCH 13/13] lint --- lib/external/s3.js | 10 +++++----- test/e2e/s3/test.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/external/s3.js b/lib/external/s3.js index a682138ce..fe56e7248 100644 --- a/lib/external/s3.js +++ b/lib/external/s3.js @@ -102,11 +102,11 @@ const init = (config) => { const wrappedOrUnhandled = (err, operation, blobOrBlobs) => { const details = { amzRequestId: err.requestid, operation }; - console.log('s3.wrappedOrUnhandled()', 'err:', err); - console.log('s3.wrappedOrUnhandled()', 'err.name:', err.name); - console.log('s3.wrappedOrUnhandled()', 'err.code:', err.code); - console.log('s3.wrappedOrUnhandled()', 'err.message:', err.message); - console.log('s3.wrappedOrUnhandled()', 'err.props:', Object.keys(err)); + console.log('s3.wrappedOrUnhandled()', 'err:', err); // eslint-disable-line no-console + console.log('s3.wrappedOrUnhandled()', 'err.name:', err.name); // eslint-disable-line no-console + console.log('s3.wrappedOrUnhandled()', 'err.code:', err.code); // eslint-disable-line no-console + console.log('s3.wrappedOrUnhandled()', 'err.message:', err.message); // eslint-disable-line no-console + console.log('s3.wrappedOrUnhandled()', 'err.props:', Object.keys(err)); // eslint-disable-line no-console if (isErrUpstream(err)) { if (Array.isArray(blobOrBlobs)) details.blobIds = blobOrBlobs.map(blob => blob.id); diff --git a/test/e2e/s3/test.js b/test/e2e/s3/test.js index d99e438af..026cfad54 100644 --- a/test/e2e/s3/test.js +++ b/test/e2e/s3/test.js @@ -330,7 +330,7 @@ describe('s3 support', () => { }); after(() => { - if(server) return new Promise(resolve => server.close(resolve)); + if(server) return new Promise(resolve => server.close(resolve)); // eslint-disable-line no-promise-executor-return }); it('should handle upload failure gracefully', async () => {