From f7dd3c152144f5f45bc7a8ea1770c7a115326493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=9F=20=E7=94=84?= Date: Sun, 11 Oct 2026 05:26:53 +0000 Subject: [PATCH 1/6] fix(graphile-cache): wait for schema build readiness --- .github/workflows/run-tests.yaml | 2 +- .../src/__tests__/build-readiness.test.ts | 178 ++++++++++++ .../src/__tests__/create-instance.test.ts | 265 ++++++++++++++++++ .../__tests__/release-failed-build.test.ts | 112 ++++++++ .../graphile-cache/src/build-readiness.ts | 39 +++ .../graphile-cache/src/create-instance.ts | 17 +- .../src/release-failed-build.ts | 38 +++ 7 files changed, 648 insertions(+), 3 deletions(-) create mode 100644 graphile/graphile-cache/src/__tests__/build-readiness.test.ts create mode 100644 graphile/graphile-cache/src/__tests__/create-instance.test.ts create mode 100644 graphile/graphile-cache/src/__tests__/release-failed-build.test.ts create mode 100644 graphile/graphile-cache/src/build-readiness.ts create mode 100644 graphile/graphile-cache/src/release-failed-build.ts diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index eda7905997..cbe9a3f38c 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -113,7 +113,7 @@ jobs: - batch: graphql packages: 'graphql/query graphql/codegen' - batch: graphile-unit - packages: 'graphile/graphile-plugin-utils graphile/graphile-realtime-subscriptions graphile/graphile-sql-expression-validator graphile/graphile-upload-plugin graphile/graphile-storage-registry' + packages: 'graphile/graphile-plugin-utils graphile/graphile-realtime-subscriptions graphile/graphile-sql-expression-validator graphile/graphile-upload-plugin graphile/graphile-storage-registry graphile/graphile-cache' - batch: agentic packages: 'agentic/protocol agentic/agentic-kit agentic/agent agentic/harness agentic/chat agentic/cli agentic/db-tools agentic/pi agentic/dsh agentic/react agentic/agentic-server agentic/anthropic agentic/openai agentic/ollama agentic/run-log agentic/metering agentic/agent-conversation agentic/pi-host agentic/run-log-client agentic/run-log-gate agentic/machine-protocol agentic/machine-runner agentic/agent-cli' - batch: pgpm-unit diff --git a/graphile/graphile-cache/src/__tests__/build-readiness.test.ts b/graphile/graphile-cache/src/__tests__/build-readiness.test.ts new file mode 100644 index 0000000000..7f608407ac --- /dev/null +++ b/graphile/graphile-cache/src/__tests__/build-readiness.test.ts @@ -0,0 +1,178 @@ +import { awaitGraphileBuildReadiness } from '../build-readiness'; + +const deferred = () => { + let resolve!: () => void; + let reject!: (error: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +}; + +// Give unhandled rejections a full event-loop turn to surface in Jest. +const nextTurn = (): Promise => new Promise((resolve) => setImmediate(resolve)); + +describe('awaitGraphileBuildReadiness', () => { + it.each(['schema', 'ready'] as const)('waits when %s is the last prerequisite', async (last) => { + const schema = deferred(); + const attachment = deferred(); + const readiness = deferred(); + const ready = jest.fn(() => readiness.promise); + const release = jest.fn(); + let resolved = false; + const build = awaitGraphileBuildReadiness({ + schemaResult: schema.promise, + addTo: () => attachment.promise, + ready, + release, + }); + const observed = build.then(() => { resolved = true; }); + + await nextTurn(); + expect(ready).not.toHaveBeenCalled(); + attachment.resolve(); + (last === 'schema' ? readiness : schema).resolve(); + await nextTurn(); + expect(ready).toHaveBeenCalledTimes(1); + expect(resolved).toBe(false); + + (last === 'schema' ? schema : readiness).resolve(); + await expect(build).resolves.toBeUndefined(); + await observed; + expect(release).not.toHaveBeenCalled(); + }); + + it('accepts synchronous void operations', async () => { + await expect(awaitGraphileBuildReadiness({ + schemaResult: Promise.resolve(), + addTo: () => {}, + ready: () => {}, + release: () => {}, + })).resolves.toBeUndefined(); + }); + + it.each([ + ['addTo', 'throw', 'resolve'], + ['addTo', 'reject', 'reject'], + ['ready', 'throw', 'reject'], + ['ready', 'reject', 'resolve'], + ] as const)('waits for schema after %s %s (schema will %s)', async (stage, mode, schemaResult) => { + const schema = deferred(); + const cleanup = deferred(); + const failure = new Error(`${stage} failed`); + const fail = (): void | Promise => { + if (mode === 'throw') throw failure; + return Promise.reject(failure); + }; + const ready = jest.fn(stage === 'ready' ? fail : () => {}); + const release = jest.fn(() => cleanup.promise); + let rejected = false; + const observed = awaitGraphileBuildReadiness({ + schemaResult: schema.promise, + addTo: stage === 'addTo' ? fail : () => {}, + ready, + release, + }).catch((error: unknown) => { + rejected = true; + return error; + }); + + await nextTurn(); + expect(release).not.toHaveBeenCalled(); + if (stage === 'addTo') expect(ready).not.toHaveBeenCalled(); + if (schemaResult === 'resolve') schema.resolve(); + else schema.reject(new Error('schema also failed')); + await nextTurn(); + expect(release).toHaveBeenCalledTimes(1); + expect(rejected).toBe(false); + + cleanup.resolve(); + expect(await observed).toBe(failure); + }); + + it.each(['resolve', 'reject'] as const)('observes early schema failure while addTo will %s', async (attachmentResult) => { + const schema = deferred(); + const attachment = deferred(); + const schemaFailure = new Error('schema failed'); + const attachmentFailure = new Error('attachment failed'); + const release = jest.fn(); + const observed = awaitGraphileBuildReadiness({ + schemaResult: schema.promise, + addTo: () => attachment.promise, + ready: () => {}, + release, + }).catch((error: unknown) => error); + + schema.reject(schemaFailure); + await nextTurn(); + expect(release).not.toHaveBeenCalled(); + if (attachmentResult === 'resolve') attachment.resolve(); + else attachment.reject(attachmentFailure); + + expect(await observed).toBe(attachmentResult === 'resolve' ? schemaFailure : attachmentFailure); + expect(release).toHaveBeenCalledTimes(1); + }); + + it('observes both schema and Grafserv rejections', async () => { + const schemaFailure = new Error('schema failed'); + const readyFailure = new Error('Grafserv failed'); + const release = jest.fn(); + await expect(awaitGraphileBuildReadiness({ + schemaResult: Promise.reject(schemaFailure), + addTo: () => {}, + ready: () => Promise.reject(readyFailure), + release, + })).rejects.toBe(readyFailure); + await nextTurn(); + expect(release).toHaveBeenCalledTimes(1); + }); + + it('waits for Grafserv to settle before releasing a failed schema', async () => { + const ready = deferred(); + const failure = new Error('schema failed'); + const release = jest.fn(); + const observed = awaitGraphileBuildReadiness({ + schemaResult: Promise.reject(failure), + addTo: () => {}, + ready: () => ready.promise, + release, + }).catch((error: unknown) => error); + + await nextTurn(); + expect(release).not.toHaveBeenCalled(); + ready.resolve(); + expect(await observed).toBe(failure); + expect(release).toHaveBeenCalledTimes(1); + }); + + it.each(['throw', 'reject'] as const)('retains the original error when release will %s', async (mode) => { + const failure = new Error('schema failed'); + const cleanupFailure = new Error('release failed'); + const onReleaseError = jest.fn(); + await expect(awaitGraphileBuildReadiness({ + schemaResult: Promise.reject(failure), + addTo: () => {}, + ready: () => {}, + release: () => { + if (mode === 'throw') throw cleanupFailure; + return Promise.reject(cleanupFailure); + }, + onReleaseError, + })).rejects.toMatchObject({ errors: [failure, cleanupFailure], cause: failure }); + expect(onReleaseError).toHaveBeenCalledWith(cleanupFailure); + }); + + it('includes a reporting failure without replacing the original cause', async () => { + const failure = new Error('schema failed'); + const cleanupFailure = new Error('release failed'); + const reportingFailure = new Error('reporting failed'); + await expect(awaitGraphileBuildReadiness({ + schemaResult: Promise.reject(failure), + addTo: () => {}, + ready: () => {}, + release: () => { throw cleanupFailure; }, + onReleaseError: () => { throw reportingFailure; }, + })).rejects.toMatchObject({ errors: [failure, cleanupFailure, reportingFailure], cause: failure }); + }); +}); diff --git a/graphile/graphile-cache/src/__tests__/create-instance.test.ts b/graphile/graphile-cache/src/__tests__/create-instance.test.ts new file mode 100644 index 0000000000..4fc2384502 --- /dev/null +++ b/graphile/graphile-cache/src/__tests__/create-instance.test.ts @@ -0,0 +1,265 @@ +import { createGraphileInstance } from '../create-instance'; + +jest.mock('postgraphile', () => ({ + postgraphile: (...args: unknown[]) => mockPostgraphile(...args), +})); + +jest.mock('express', () => ({ + __esModule: true, + default: (...args: unknown[]) => mockExpress(...args), +})); + +jest.mock('node:http', () => ({ + createServer: (...args: unknown[]) => mockCreateServer(...args), +})); + +jest.mock('grafserv/express/v4', () => ({ grafserv: jest.fn() })); + +jest.mock('graphile-realtime-subscriptions', () => ({ + RealtimeManager: function (...args: unknown[]) { + return mockCreateRealtimeManager(...args); + }, +})); + +jest.mock('@pgpmjs/logger', () => ({ + Logger: function () { + return { + info: (...args: unknown[]) => mockInfo(...args), + warn: (...args: unknown[]) => mockWarn(...args), + error: (...args: unknown[]) => mockError(...args), + }; + }, +})); + +const mockPostgraphile = jest.fn(); +const mockExpress = jest.fn(); +const mockCreateServer = jest.fn(); +const mockCreateRealtimeManager = jest.fn(); +const mockInfo = jest.fn(); +const mockWarn = jest.fn(); +const mockError = jest.fn(); + +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + +const flushAsyncWork = () => new Promise((resolve) => setImmediate(resolve)); + +describe('createGraphileInstance', () => { + const preset = { grafserv: { graphqlPath: '/graphql' } }; + const cacheKey = 'readiness-test'; + const handler = jest.fn(); + const httpServer = { close: jest.fn() }; + const pgSubscriber = {}; + const pool = {}; + const releaseService = jest.fn(); + const schemaResult = { schema: {} }; + const manager = { start: jest.fn() }; + const serv = { + addTo: jest.fn(), + ready: jest.fn(), + }; + const pgl = { + createServ: jest.fn(), + getSchemaResult: jest.fn(), + getResolvedPreset: jest.fn(), + release: jest.fn(), + }; + + beforeEach(() => { + jest.resetAllMocks(); + mockPostgraphile.mockReturnValue(pgl); + mockExpress.mockReturnValue(handler); + mockCreateServer.mockReturnValue(httpServer); + mockCreateRealtimeManager.mockReturnValue(manager); + pgl.createServ.mockReturnValue(serv); + pgl.getSchemaResult.mockResolvedValue(schemaResult); + pgl.getResolvedPreset.mockReturnValue({ + pgServices: [{ pgSubscriber, adaptorSettings: { pool }, release: releaseService }], + }); + pgl.release.mockResolvedValue(undefined); + releaseService.mockResolvedValue(undefined); + serv.addTo.mockResolvedValue(undefined); + serv.ready.mockResolvedValue(undefined); + manager.start.mockResolvedValue(undefined); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it.each(['schema', 'HTTP'] as const)( + 'waits for schema, HTTP and Grafserv before realtime when %s completes first', + async (first) => { + const schema = deferred(); + const attachment = deferred(); + const ready = deferred(); + const started = deferred(); + pgl.getSchemaResult.mockReturnValue(schema.promise); + serv.addTo.mockReturnValue(attachment.promise); + serv.ready.mockReturnValue(ready.promise); + manager.start.mockReturnValue(started.promise); + + const settled = jest.fn(); + const instance = createGraphileInstance({ preset, cacheKey, enableRealtime: true }); + void instance.then(settled, settled); + await flushAsyncWork(); + + expect(mockPostgraphile).toHaveBeenCalledWith(preset); + expect(pgl.getSchemaResult).toHaveBeenCalledTimes(1); + expect(mockCreateServer).toHaveBeenCalledWith(handler); + expect(serv.addTo).toHaveBeenCalledWith(handler, httpServer); + expect(serv.ready).not.toHaveBeenCalled(); + + if (first === 'schema') { + schema.resolve(schemaResult); + } else { + attachment.resolve(); + } + await flushAsyncWork(); + expect(serv.ready).toHaveBeenCalledTimes(first === 'HTTP' ? 1 : 0); + expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); + expect(settled).not.toHaveBeenCalled(); + + schema.resolve(schemaResult); + attachment.resolve(); + await flushAsyncWork(); + expect(serv.ready).toHaveBeenCalledTimes(1); + expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); + expect(settled).not.toHaveBeenCalled(); + + ready.resolve(); + await flushAsyncWork(); + expect(mockCreateRealtimeManager).toHaveBeenCalledWith({ + pgSubscriber, + pool, + nodeId: `graphile-cache:${cacheKey}`, + schema: 'realtime_public', + }); + expect(manager.start).toHaveBeenCalledTimes(1); + expect(settled).not.toHaveBeenCalled(); + + started.resolve(); + await expect(instance).resolves.toEqual({ + pgl, + serv, + handler, + httpServer, + cacheKey, + createdAt: expect.any(Number), + realtimeManager: manager, + }); + expect(pgl.release).not.toHaveBeenCalled(); + expect(releaseService).not.toHaveBeenCalled(); + }, + ); + + it('accepts a synchronous schema result and void HTTP and Grafserv readiness results', async () => { + pgl.getSchemaResult.mockReturnValue(schemaResult); + serv.addTo.mockReturnValue(undefined); + serv.ready.mockReturnValue(undefined); + + await expect(createGraphileInstance({ preset, cacheKey })).resolves.toMatchObject({ + pgl, + serv, + handler, + httpServer, + cacheKey, + }); + expect(serv.ready).toHaveBeenCalledTimes(1); + expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); + expect(pgl.release).not.toHaveBeenCalled(); + expect(releaseService).not.toHaveBeenCalled(); + }); + + it('waits for schema settlement and cleanup while preserving the original HTTP failure', async () => { + const schema = deferred(); + const attachment = deferred(); + const released = deferred(); + const attachmentError = new Error('HTTP attachment failed'); + const schemaError = new Error('schema failed after HTTP attachment'); + pgl.getSchemaResult.mockReturnValue(schema.promise); + serv.addTo.mockReturnValue(attachment.promise); + pgl.release.mockReturnValue(released.promise); + + const settled = jest.fn(); + const instance = createGraphileInstance({ preset, cacheKey, enableRealtime: true }); + void instance.then(settled, settled); + const rejection = expect(instance).rejects.toBe(attachmentError); + await flushAsyncWork(); + attachment.reject(attachmentError); + await flushAsyncWork(); + expect(pgl.release).not.toHaveBeenCalled(); + expect(releaseService).not.toHaveBeenCalled(); + expect(settled).not.toHaveBeenCalled(); + + schema.reject(schemaError); + await flushAsyncWork(); + expect(pgl.release).toHaveBeenCalledTimes(1); + expect(releaseService).not.toHaveBeenCalled(); + expect(settled).not.toHaveBeenCalled(); + + released.resolve(); + await rejection; + expect(releaseService).toHaveBeenCalledTimes(1); + expect(mockError).not.toHaveBeenCalled(); + expect(serv.ready).not.toHaveBeenCalled(); + expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); + }); + + it('reports cleanup failure with the original build error as its cause', async () => { + const buildError = new Error('Grafserv readiness failed'); + const releaseError = new Error('PostGraphile release failed'); + serv.ready.mockRejectedValue(buildError); + pgl.release.mockRejectedValue(releaseError); + + await expect(createGraphileInstance({ preset, cacheKey })).rejects.toMatchObject({ + cause: buildError, + errors: [buildError, releaseError], + }); + expect(mockError).toHaveBeenCalledWith( + `Failed to release PostGraphile[${cacheKey}] after build failure:`, + releaseError, + ); + expect(releaseService).toHaveBeenCalledTimes(1); + }); + + it('still returns a ready instance when realtime startup fails', async () => { + const realtimeError = new Error('realtime unavailable'); + manager.start.mockRejectedValue(realtimeError); + + const entry = await createGraphileInstance({ preset, cacheKey, enableRealtime: true }); + + expect(entry.pgl).toBe(pgl); + expect(entry.realtimeManager).toBeUndefined(); + expect(mockError).toHaveBeenCalledWith( + `Failed to start RealtimeManager for PostGraphile[${cacheKey}]:`, + realtimeError, + ); + expect(pgl.release).not.toHaveBeenCalled(); + expect(releaseService).not.toHaveBeenCalled(); + }); + + it.each([ + { pgService: { adaptorSettings: { pool } }, missing: 'pgSubscriber' }, + { pgService: { pgSubscriber }, missing: 'pool in pgService' }, + ])('still skips realtime when $missing is absent', async ({ pgService, missing }) => { + pgl.getResolvedPreset.mockReturnValue({ pgServices: [pgService] }); + + const entry = await createGraphileInstance({ preset, cacheKey, enableRealtime: true }); + + expect(entry.pgl).toBe(pgl); + expect(entry.realtimeManager).toBeUndefined(); + expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); + expect(mockWarn).toHaveBeenCalledWith( + `PostGraphile[${cacheKey}] has no ${missing} — RealtimeManager will not be started`, + ); + expect(pgl.release).not.toHaveBeenCalled(); + }); +}); diff --git a/graphile/graphile-cache/src/__tests__/release-failed-build.test.ts b/graphile/graphile-cache/src/__tests__/release-failed-build.test.ts new file mode 100644 index 0000000000..14b15fee68 --- /dev/null +++ b/graphile/graphile-cache/src/__tests__/release-failed-build.test.ts @@ -0,0 +1,112 @@ +import { releaseFailedBuild } from '../release-failed-build'; + +type FailedBuild = Parameters[0]; + +const makeService = ( + release?: () => void | PromiseLike +) => ({ release }); + +const makeInstance = ( + services?: readonly ReturnType[], + release: FailedBuild['release'] = jest.fn() +): FailedBuild => ({ + release, + getResolvedPreset: () => ({ pgServices: services }), +}); + +describe('releaseFailedBuild', () => { + it('waits for PostGraphile, then releases services in reverse order', async () => { + const events: string[] = []; + let finishPostGraphile!: () => void; + const postGraphileReleased = new Promise((resolve) => { + finishPostGraphile = resolve; + }); + let finishSecond!: () => void; + const secondReleased = new Promise((resolve) => { + finishSecond = resolve; + }); + const first = makeService(() => { events.push('first'); }); + const second = makeService(async () => { + events.push('second'); + await secondReleased; + }); + const release = jest.fn(async () => { + events.push('postgraphile'); + await postGraphileReleased; + }); + + const result = releaseFailedBuild(makeInstance([first, second], release)); + await Promise.resolve(); + expect(events).toEqual(['postgraphile']); + + finishPostGraphile(); + await new Promise((resolve) => setImmediate(resolve)); + expect(events).toEqual(['postgraphile', 'second']); + + finishSecond(); + await expect(result).resolves.toBeUndefined(); + expect(events).toEqual(['postgraphile', 'second', 'first']); + expect(release).toHaveBeenCalledTimes(1); + }); + + it('snapshots and deduplicates services before releasing PostGraphile', async () => { + const releaseService = jest.fn(); + const service = makeService(releaseService); + const services = [service, makeService(), service]; + const release = jest.fn(() => { services.length = 0; }); + + await releaseFailedBuild(makeInstance(services, release)); + + expect(releaseService).toHaveBeenCalledTimes(1); + }); + + it('releases PostGraphile when the preset has no services', async () => { + const release = jest.fn(); + + await expect(releaseFailedBuild(makeInstance(undefined, release))).resolves.toBeUndefined(); + + expect(release).toHaveBeenCalledTimes(1); + }); + + it('preserves a single release error and still attempts remaining services', async () => { + const error = new Error('service release failed'); + const releaseFirst = jest.fn(); + const first = makeService(releaseFirst); + const second = makeService(() => { throw error; }); + + await expect(releaseFailedBuild(makeInstance([first, second]))).rejects.toBe(error); + + expect(releaseFirst).toHaveBeenCalledTimes(1); + }); + + it('collects synchronous and asynchronous errors without skipping any releases', async () => { + const postGraphileError = new Error('PostGraphile release failed'); + const firstError = new Error('first service release failed'); + const secondError = new Error('second service release failed'); + const events: string[] = []; + const first = makeService(() => { + events.push('first'); + throw firstError; + }); + const second = makeService(async () => { + events.push('second'); + throw secondError; + }); + const release = () => { + events.push('postgraphile'); + throw postGraphileError; + }; + + const error = await releaseFailedBuild(makeInstance([first, second], release)).catch( + (caught: unknown) => caught + ); + + expect(events).toEqual(['postgraphile', 'second', 'first']); + expect(error).toBeInstanceOf(AggregateError); + if (!(error instanceof AggregateError)) throw new Error('Expected aggregate cleanup error'); + expect(error.errors).toHaveLength(3); + expect(error.errors[0]).toBe(postGraphileError); + expect(error.errors[1]).toBe(secondError); + expect(error.errors[2]).toBe(firstError); + }); +}); diff --git a/graphile/graphile-cache/src/build-readiness.ts b/graphile/graphile-cache/src/build-readiness.ts new file mode 100644 index 0000000000..347d8f34e1 --- /dev/null +++ b/graphile/graphile-cache/src/build-readiness.ts @@ -0,0 +1,39 @@ +export interface GraphileBuildReadiness { + schemaResult: Promise; + addTo(): void | PromiseLike; + ready(): void | PromiseLike; + release(): void | PromiseLike; + onReleaseError?(error: unknown): void; +} + +/** Wait for schema construction, HTTP attachment and Grafserv readiness. */ +export const awaitGraphileBuildReadiness = async ( + build: GraphileBuildReadiness +): Promise => { + // Observe schema rejection immediately, even while addTo is still pending. + // Keeping it as an outcome also lets failure cleanup wait for schema work. + const schemaOutcome = build.schemaResult.then( + () => ({ ready: true as const }), + (error: unknown) => ({ ready: false as const, error }) + ); + try { + await build.addTo(); + const [schema] = await Promise.all([schemaOutcome, build.ready()]); + if ('error' in schema) throw schema.error; + } catch (error) { + // Schema gathering may still be using the services we are about to release. + await schemaOutcome; + try { + await build.release(); + } catch (releaseError) { + const failures = [error, releaseError]; + try { + build.onReleaseError?.(releaseError); + } catch (observerError) { + failures.push(observerError); + } + throw new AggregateError(failures, 'Graphile build and cleanup failed', { cause: error }); + } + throw error; + } +}; diff --git a/graphile/graphile-cache/src/create-instance.ts b/graphile/graphile-cache/src/create-instance.ts index a522701720..783185b823 100644 --- a/graphile/graphile-cache/src/create-instance.ts +++ b/graphile/graphile-cache/src/create-instance.ts @@ -6,7 +6,9 @@ import { grafserv } from 'grafserv/express/v4'; import { RealtimeManager } from 'graphile-realtime-subscriptions'; import { postgraphile } from 'postgraphile'; +import { awaitGraphileBuildReadiness } from './build-readiness'; import type { GraphileCacheEntry } from './graphile-cache'; +import { releaseFailedBuild } from './release-failed-build'; const log = new Logger('graphile-cache:create'); @@ -30,6 +32,9 @@ interface GraphileInstanceOptions { * * Callers are responsible for building the `GraphileConfig.Preset` (including * pgServices, grafserv options, grafast context, etc.) before passing it here. + * On a readiness failure, this factory releases PostGraphile and the preset's + * services after schema construction settles; externally supplied pools stay + * owned by the caller. * * When `enableRealtime` is true, a RealtimeManager is created that bridges * cursor-tracked events from `drain_changes()` into the PostGraphile @@ -47,8 +52,16 @@ export const createGraphileInstance = async ( const handler = express(); const httpServer = createServer(handler); - await serv.addTo(handler, httpServer); - await serv.ready(); + await awaitGraphileBuildReadiness({ + // The upstream API returns a schema result object, possibly asynchronously. + schemaResult: Promise.resolve(pgl.getSchemaResult()).then(() => {}), + addTo: () => serv.addTo(handler, httpServer), + ready: () => serv.ready(), + release: () => releaseFailedBuild(pgl), + onReleaseError: (releaseError) => { + log.error(`Failed to release PostGraphile[${cacheKey}] after build failure:`, releaseError); + }, + }); const entry: GraphileCacheEntry = { pgl, diff --git a/graphile/graphile-cache/src/release-failed-build.ts b/graphile/graphile-cache/src/release-failed-build.ts new file mode 100644 index 0000000000..1a9b64c6fb --- /dev/null +++ b/graphile/graphile-cache/src/release-failed-build.ts @@ -0,0 +1,38 @@ +interface ReleasableService { + release?(): void | PromiseLike; +} + +interface FailedBuild { + release(): void | PromiseLike; + getResolvedPreset(): { pgServices?: readonly ReleasableService[] }; +} + +/** + * Release a failed build after its schema work has settled. PostGraphile releases + * Grafserv and schema watching; each service releases only its own resources. + */ +export const releaseFailedBuild = async ( + pgl: FailedBuild +): Promise => { + const services = [...new Set(pgl.getResolvedPreset().pgServices ?? [])]; + const errors: unknown[] = []; + + try { + await pgl.release(); + } catch (error) { + errors.push(error); + } + + for (const service of services.reverse()) { + try { + await service.release?.(); + } catch (error) { + errors.push(error); + } + } + + if (errors.length === 1) throw errors[0]; + if (errors.length > 1) { + throw new AggregateError(errors, 'Failed to release Graphile build resources'); + } +}; From eee9e366d32390d606f2b9677065b04ac2cbaa2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=9F=20=E7=94=84?= Date: Sun, 11 Oct 2026 05:40:23 +0000 Subject: [PATCH 2/6] refactor(graphile-cache): organize utilities under src/utils --- .../src/__tests__/build-readiness.test.ts | 2 +- .../__tests__/release-failed-build.test.ts | 2 +- .../graphile-cache/src/create-instance.ts | 4 +- graphile/graphile-cache/src/index.ts | 2 +- .../graphile-cache/src/module-config-cache.ts | 68 +------------------ .../src/{ => utils}/build-readiness.ts | 0 .../src/utils/module-config-cache.ts | 65 ++++++++++++++++++ .../src/{ => utils}/release-failed-build.ts | 0 8 files changed, 73 insertions(+), 70 deletions(-) rename graphile/graphile-cache/src/{ => utils}/build-readiness.ts (100%) create mode 100644 graphile/graphile-cache/src/utils/module-config-cache.ts rename graphile/graphile-cache/src/{ => utils}/release-failed-build.ts (100%) diff --git a/graphile/graphile-cache/src/__tests__/build-readiness.test.ts b/graphile/graphile-cache/src/__tests__/build-readiness.test.ts index 7f608407ac..a23a912b9e 100644 --- a/graphile/graphile-cache/src/__tests__/build-readiness.test.ts +++ b/graphile/graphile-cache/src/__tests__/build-readiness.test.ts @@ -1,4 +1,4 @@ -import { awaitGraphileBuildReadiness } from '../build-readiness'; +import { awaitGraphileBuildReadiness } from '../utils/build-readiness'; const deferred = () => { let resolve!: () => void; diff --git a/graphile/graphile-cache/src/__tests__/release-failed-build.test.ts b/graphile/graphile-cache/src/__tests__/release-failed-build.test.ts index 14b15fee68..4dfe4e1488 100644 --- a/graphile/graphile-cache/src/__tests__/release-failed-build.test.ts +++ b/graphile/graphile-cache/src/__tests__/release-failed-build.test.ts @@ -1,4 +1,4 @@ -import { releaseFailedBuild } from '../release-failed-build'; +import { releaseFailedBuild } from '../utils/release-failed-build'; type FailedBuild = Parameters[0]; diff --git a/graphile/graphile-cache/src/create-instance.ts b/graphile/graphile-cache/src/create-instance.ts index 783185b823..1aba497480 100644 --- a/graphile/graphile-cache/src/create-instance.ts +++ b/graphile/graphile-cache/src/create-instance.ts @@ -6,9 +6,9 @@ import { grafserv } from 'grafserv/express/v4'; import { RealtimeManager } from 'graphile-realtime-subscriptions'; import { postgraphile } from 'postgraphile'; -import { awaitGraphileBuildReadiness } from './build-readiness'; import type { GraphileCacheEntry } from './graphile-cache'; -import { releaseFailedBuild } from './release-failed-build'; +import { awaitGraphileBuildReadiness } from './utils/build-readiness'; +import { releaseFailedBuild } from './utils/release-failed-build'; const log = new Logger('graphile-cache:create'); diff --git a/graphile/graphile-cache/src/index.ts b/graphile/graphile-cache/src/index.ts index 9a845fafe3..fd06d960c0 100644 --- a/graphile/graphile-cache/src/index.ts +++ b/graphile/graphile-cache/src/index.ts @@ -26,4 +26,4 @@ export { export { createGraphileInstance } from './create-instance'; // Generic module config cache for plugin lookups -export { ModuleConfigCache, ModuleConfigCacheOptions } from './module-config-cache'; +export { ModuleConfigCache, ModuleConfigCacheOptions } from './utils/module-config-cache'; diff --git a/graphile/graphile-cache/src/module-config-cache.ts b/graphile/graphile-cache/src/module-config-cache.ts index 63af02a93e..5449b10777 100644 --- a/graphile/graphile-cache/src/module-config-cache.ts +++ b/graphile/graphile-cache/src/module-config-cache.ts @@ -1,65 +1,3 @@ -/** - * ModuleConfigCache — generic LRU cache for module config lookups - * - * Provides a bounded, TTL-based cache for runtime config discovery - * (billing config, agent discovery, inference log info, etc.). - * - * Wraps lru-cache with: - * - LRU eviction (bounded memory) - * - TTL-based expiry - * - Named logging for debugging - * - clear() hook for future LISTEN/NOTIFY invalidation - */ - -import { Logger } from '@pgpmjs/logger'; -import { LRUCache } from 'lru-cache'; - -export interface ModuleConfigCacheOptions { - /** Cache name (used in log prefix) */ - name: string; - /** Max entries before LRU eviction (default: 100) */ - max?: number; - /** TTL in milliseconds (default: 60_000) */ - ttlMs?: number; -} - -export class ModuleConfigCache { - private cache: LRUCache; - private log: Logger; - readonly name: string; - - constructor(opts: ModuleConfigCacheOptions) { - this.name = opts.name; - this.log = new Logger(`cache:${opts.name}`); - this.cache = new LRUCache({ - max: opts.max ?? 100, - ttl: opts.ttlMs ?? 60_000, - updateAgeOnGet: true, - }); - } - - get(key: string): T | undefined { - return this.cache.get(key); - } - - set(key: string, value: T): void { - this.cache.set(key, value); - } - - delete(key: string): boolean { - return this.cache.delete(key); - } - - clear(): void { - this.log.debug(`Clearing all entries (size=${this.cache.size})`); - this.cache.clear(); - } - - get size(): number { - return this.cache.size; - } - - has(key: string): boolean { - return this.cache.has(key); - } -} +// Preserve the existing graphile-cache/module-config-cache import path. +export type { ModuleConfigCacheOptions } from './utils/module-config-cache'; +export { ModuleConfigCache } from './utils/module-config-cache'; diff --git a/graphile/graphile-cache/src/build-readiness.ts b/graphile/graphile-cache/src/utils/build-readiness.ts similarity index 100% rename from graphile/graphile-cache/src/build-readiness.ts rename to graphile/graphile-cache/src/utils/build-readiness.ts diff --git a/graphile/graphile-cache/src/utils/module-config-cache.ts b/graphile/graphile-cache/src/utils/module-config-cache.ts new file mode 100644 index 0000000000..63af02a93e --- /dev/null +++ b/graphile/graphile-cache/src/utils/module-config-cache.ts @@ -0,0 +1,65 @@ +/** + * ModuleConfigCache — generic LRU cache for module config lookups + * + * Provides a bounded, TTL-based cache for runtime config discovery + * (billing config, agent discovery, inference log info, etc.). + * + * Wraps lru-cache with: + * - LRU eviction (bounded memory) + * - TTL-based expiry + * - Named logging for debugging + * - clear() hook for future LISTEN/NOTIFY invalidation + */ + +import { Logger } from '@pgpmjs/logger'; +import { LRUCache } from 'lru-cache'; + +export interface ModuleConfigCacheOptions { + /** Cache name (used in log prefix) */ + name: string; + /** Max entries before LRU eviction (default: 100) */ + max?: number; + /** TTL in milliseconds (default: 60_000) */ + ttlMs?: number; +} + +export class ModuleConfigCache { + private cache: LRUCache; + private log: Logger; + readonly name: string; + + constructor(opts: ModuleConfigCacheOptions) { + this.name = opts.name; + this.log = new Logger(`cache:${opts.name}`); + this.cache = new LRUCache({ + max: opts.max ?? 100, + ttl: opts.ttlMs ?? 60_000, + updateAgeOnGet: true, + }); + } + + get(key: string): T | undefined { + return this.cache.get(key); + } + + set(key: string, value: T): void { + this.cache.set(key, value); + } + + delete(key: string): boolean { + return this.cache.delete(key); + } + + clear(): void { + this.log.debug(`Clearing all entries (size=${this.cache.size})`); + this.cache.clear(); + } + + get size(): number { + return this.cache.size; + } + + has(key: string): boolean { + return this.cache.has(key); + } +} diff --git a/graphile/graphile-cache/src/release-failed-build.ts b/graphile/graphile-cache/src/utils/release-failed-build.ts similarity index 100% rename from graphile/graphile-cache/src/release-failed-build.ts rename to graphile/graphile-cache/src/utils/release-failed-build.ts From 3fd84f80c1819e7113ab75a85f33bda412e2ba86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=9F=20=E7=94=84?= Date: Sun, 11 Oct 2026 05:44:36 +0000 Subject: [PATCH 3/6] refactor(graphile-cache): remove legacy module cache re-export --- graphile/graphile-cache/src/module-config-cache.ts | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 graphile/graphile-cache/src/module-config-cache.ts diff --git a/graphile/graphile-cache/src/module-config-cache.ts b/graphile/graphile-cache/src/module-config-cache.ts deleted file mode 100644 index 5449b10777..0000000000 --- a/graphile/graphile-cache/src/module-config-cache.ts +++ /dev/null @@ -1,3 +0,0 @@ -// Preserve the existing graphile-cache/module-config-cache import path. -export type { ModuleConfigCacheOptions } from './utils/module-config-cache'; -export { ModuleConfigCache } from './utils/module-config-cache'; From a707c761a386e7cd0e4379a25bab8981713eb058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=9F=20=E7=94=84?= Date: Sun, 11 Oct 2026 06:02:40 +0000 Subject: [PATCH 4/6] refactor(graphile-cache): await schema readiness with async helper --- .../src/__tests__/create-instance.test.ts | 49 +++++++++++++++++++ .../graphile-cache/src/create-instance.ts | 6 ++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/graphile/graphile-cache/src/__tests__/create-instance.test.ts b/graphile/graphile-cache/src/__tests__/create-instance.test.ts index 4fc2384502..1700c159a9 100644 --- a/graphile/graphile-cache/src/__tests__/create-instance.test.ts +++ b/graphile/graphile-cache/src/__tests__/create-instance.test.ts @@ -178,6 +178,55 @@ describe('createGraphileInstance', () => { expect(releaseService).not.toHaveBeenCalled(); }); + it.each(['throws synchronously', 'rejects immediately'] as const)( + 'observes schema failure that %s while HTTP attachment is pending, then awaits cleanup', + async (failure) => { + const schemaError = new Error('schema failed before HTTP attachment'); + const attachment = deferred(); + const released = deferred(); + const serviceReleased = deferred(); + if (failure === 'throws synchronously') { + pgl.getSchemaResult.mockImplementationOnce(() => { throw schemaError; }); + } else { + pgl.getSchemaResult.mockRejectedValueOnce(schemaError); + } + serv.addTo.mockReturnValue(attachment.promise); + pgl.release.mockReturnValue(released.promise); + releaseService.mockReturnValue(serviceReleased.promise); + + const settled = jest.fn(); + const instance = createGraphileInstance({ preset, cacheKey }); + void instance.then(settled, settled); + const rejection = expect(instance).rejects.toBe(schemaError); + expect(pgl.getSchemaResult).toHaveBeenCalledTimes(1); + expect(serv.addTo).toHaveBeenCalledTimes(1); + expect(pgl.getSchemaResult.mock.invocationCallOrder[0]).toBeLessThan( + serv.addTo.mock.invocationCallOrder[0], + ); + + // Cross a macrotask boundary so an unobserved schema rejection fails the test. + await flushAsyncWork(); + expect(settled).not.toHaveBeenCalled(); + expect(pgl.release).not.toHaveBeenCalled(); + expect(releaseService).not.toHaveBeenCalled(); + + attachment.resolve(); + await flushAsyncWork(); + expect(pgl.release).toHaveBeenCalledTimes(1); + expect(releaseService).not.toHaveBeenCalled(); + expect(settled).not.toHaveBeenCalled(); + + released.resolve(); + await flushAsyncWork(); + expect(releaseService).toHaveBeenCalledTimes(1); + expect(settled).not.toHaveBeenCalled(); + + serviceReleased.resolve(); + await rejection; + expect(mockError).not.toHaveBeenCalled(); + }, + ); + it('waits for schema settlement and cleanup while preserving the original HTTP failure', async () => { const schema = deferred(); const attachment = deferred(); diff --git a/graphile/graphile-cache/src/create-instance.ts b/graphile/graphile-cache/src/create-instance.ts index 1aba497480..5481584e00 100644 --- a/graphile/graphile-cache/src/create-instance.ts +++ b/graphile/graphile-cache/src/create-instance.ts @@ -52,9 +52,11 @@ export const createGraphileInstance = async ( const handler = express(); const httpServer = createServer(handler); + const waitForSchema = async () => { + await pgl.getSchemaResult(); + }; await awaitGraphileBuildReadiness({ - // The upstream API returns a schema result object, possibly asynchronously. - schemaResult: Promise.resolve(pgl.getSchemaResult()).then(() => {}), + schemaResult: waitForSchema(), addTo: () => serv.addTo(handler, httpServer), ready: () => serv.ready(), release: () => releaseFailedBuild(pgl), From c9459841cc752c53c97f38357b7ec0935a89de63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=9F=20=E7=94=84?= Date: Sun, 11 Oct 2026 06:08:50 +0000 Subject: [PATCH 5/6] refactor(graphile-cache): express schema readiness with async blocks --- graphile/graphile-cache/src/create-instance.ts | 7 +++---- graphile/graphile-cache/src/utils/build-readiness.ts | 12 ++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/graphile/graphile-cache/src/create-instance.ts b/graphile/graphile-cache/src/create-instance.ts index 5481584e00..629fbc91f3 100644 --- a/graphile/graphile-cache/src/create-instance.ts +++ b/graphile/graphile-cache/src/create-instance.ts @@ -52,11 +52,10 @@ export const createGraphileInstance = async ( const handler = express(); const httpServer = createServer(handler); - const waitForSchema = async () => { - await pgl.getSchemaResult(); - }; await awaitGraphileBuildReadiness({ - schemaResult: waitForSchema(), + schemaResult: (async () => { + await pgl.getSchemaResult(); + })(), addTo: () => serv.addTo(handler, httpServer), ready: () => serv.ready(), release: () => releaseFailedBuild(pgl), diff --git a/graphile/graphile-cache/src/utils/build-readiness.ts b/graphile/graphile-cache/src/utils/build-readiness.ts index 347d8f34e1..afd8a4b0a1 100644 --- a/graphile/graphile-cache/src/utils/build-readiness.ts +++ b/graphile/graphile-cache/src/utils/build-readiness.ts @@ -12,10 +12,14 @@ export const awaitGraphileBuildReadiness = async ( ): Promise => { // Observe schema rejection immediately, even while addTo is still pending. // Keeping it as an outcome also lets failure cleanup wait for schema work. - const schemaOutcome = build.schemaResult.then( - () => ({ ready: true as const }), - (error: unknown) => ({ ready: false as const, error }) - ); + const schemaOutcome = (async () => { + try { + await build.schemaResult; + return { ready: true as const }; + } catch (error: unknown) { + return { ready: false as const, error }; + } + })(); try { await build.addTo(); const [schema] = await Promise.all([schemaOutcome, build.ready()]); From 49ae6daf45f8b024d374f8f207ace67c0fe9fa81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=9F=20=E7=94=84?= Date: Sun, 11 Oct 2026 07:14:31 +0000 Subject: [PATCH 6/6] refactor(graphile-cache): initialize schema before Grafserv --- .../src/__tests__/build-readiness.test.ts | 200 +++++++----------- .../src/__tests__/create-instance.test.ts | 54 ++--- .../__tests__/fixtures/sequential-runtime.cjs | 157 ++++++++++++++ .../src/__tests__/sequential-runtime.test.ts | 35 +++ .../graphile-cache/src/create-instance.ts | 11 +- .../src/utils/build-readiness.ts | 23 +- 6 files changed, 305 insertions(+), 175 deletions(-) create mode 100644 graphile/graphile-cache/src/__tests__/fixtures/sequential-runtime.cjs create mode 100644 graphile/graphile-cache/src/__tests__/sequential-runtime.test.ts diff --git a/graphile/graphile-cache/src/__tests__/build-readiness.test.ts b/graphile/graphile-cache/src/__tests__/build-readiness.test.ts index a23a912b9e..a1caa5389a 100644 --- a/graphile/graphile-cache/src/__tests__/build-readiness.test.ts +++ b/graphile/graphile-cache/src/__tests__/build-readiness.test.ts @@ -3,176 +3,120 @@ import { awaitGraphileBuildReadiness } from '../utils/build-readiness'; const deferred = () => { let resolve!: () => void; let reject!: (error: unknown) => void; - const promise = new Promise((resolvePromise, rejectPromise) => { - resolve = resolvePromise; - reject = rejectPromise; - }); + const promise = new Promise((res, rej) => { resolve = res; reject = rej; }); return { promise, resolve, reject }; }; - -// Give unhandled rejections a full event-loop turn to surface in Jest. const nextTurn = (): Promise => new Promise((resolve) => setImmediate(resolve)); describe('awaitGraphileBuildReadiness', () => { - it.each(['schema', 'ready'] as const)('waits when %s is the last prerequisite', async (last) => { + it('waits for schema, attachment and readiness in order', async () => { const schema = deferred(); const attachment = deferred(); const readiness = deferred(); - const ready = jest.fn(() => readiness.promise); + const events: string[] = []; + const schemaResult = jest.fn(() => { events.push('schema'); return schema.promise; }); + const addTo = jest.fn(() => { events.push('addTo'); return attachment.promise; }); + const ready = jest.fn(() => { events.push('ready'); return readiness.promise; }); const release = jest.fn(); - let resolved = false; - const build = awaitGraphileBuildReadiness({ - schemaResult: schema.promise, - addTo: () => attachment.promise, - ready, - release, - }); - const observed = build.then(() => { resolved = true; }); - + let complete = false; + const result = awaitGraphileBuildReadiness({ schemaResult, addTo, ready, release }); + const observed = result.then(() => { complete = true; }); await nextTurn(); - expect(ready).not.toHaveBeenCalled(); + expect(events).toEqual(['schema']); + schema.resolve(); + await nextTurn(); + expect(events).toEqual(['schema', 'addTo']); attachment.resolve(); - (last === 'schema' ? readiness : schema).resolve(); await nextTurn(); - expect(ready).toHaveBeenCalledTimes(1); - expect(resolved).toBe(false); - - (last === 'schema' ? schema : readiness).resolve(); - await expect(build).resolves.toBeUndefined(); + expect(events).toEqual(['schema', 'addTo', 'ready']); + expect(complete).toBe(false); + readiness.resolve(); await observed; + expect(schemaResult).toHaveBeenCalledTimes(1); expect(release).not.toHaveBeenCalled(); }); - it('accepts synchronous void operations', async () => { + it('accepts synchronous void callbacks', async () => { await expect(awaitGraphileBuildReadiness({ - schemaResult: Promise.resolve(), - addTo: () => {}, - ready: () => {}, - release: () => {}, + schemaResult: () => {}, addTo: () => {}, ready: () => {}, release: () => {}, })).resolves.toBeUndefined(); }); + it.each(['resolve', 'reject'] as const)( + 'does not start a failing attachment until the pending schema will %s', async (outcome) => { + const schema = deferred(); + const schemaFailure = new Error('schema failed'); + const attachmentFailure = new Error('attachment failed'); + const addTo = jest.fn(() => { throw attachmentFailure; }); + const ready = jest.fn(); + const release = jest.fn(); + const observed = awaitGraphileBuildReadiness({ + schemaResult: () => schema.promise, addTo, ready, release, + }).catch((error: unknown) => error); + await nextTurn(); + expect(addTo).not.toHaveBeenCalled(); + expect(release).not.toHaveBeenCalled(); + if (outcome === 'resolve') schema.resolve(); + else schema.reject(schemaFailure); + expect(await observed).toBe(outcome === 'resolve' ? attachmentFailure : schemaFailure); + expect(addTo).toHaveBeenCalledTimes(outcome === 'resolve' ? 1 : 0); + expect(ready).not.toHaveBeenCalled(); + expect(release).toHaveBeenCalledTimes(1); + }, + ); + it.each([ - ['addTo', 'throw', 'resolve'], - ['addTo', 'reject', 'reject'], - ['ready', 'throw', 'reject'], - ['ready', 'reject', 'resolve'], - ] as const)('waits for schema after %s %s (schema will %s)', async (stage, mode, schemaResult) => { - const schema = deferred(); - const cleanup = deferred(); + ['schema', 'throw'], ['schema', 'reject'], + ['addTo', 'throw'], ['addTo', 'reject'], + ['ready', 'throw'], ['ready', 'reject'], + ] as const)('preserves a %s %s and awaits cleanup', async (stage, mode) => { const failure = new Error(`${stage} failed`); - const fail = (): void | Promise => { - if (mode === 'throw') throw failure; - return Promise.reject(failure); - }; + const cleanup = deferred(); + const fail = () => { if (mode === 'throw') throw failure; return Promise.reject(failure); }; + const schemaResult = jest.fn(stage === 'schema' ? fail : () => {}); + const addTo = jest.fn(stage === 'addTo' ? fail : () => {}); const ready = jest.fn(stage === 'ready' ? fail : () => {}); const release = jest.fn(() => cleanup.promise); - let rejected = false; - const observed = awaitGraphileBuildReadiness({ - schemaResult: schema.promise, - addTo: stage === 'addTo' ? fail : () => {}, - ready, - release, - }).catch((error: unknown) => { - rejected = true; - return error; - }); - - await nextTurn(); - expect(release).not.toHaveBeenCalled(); - if (stage === 'addTo') expect(ready).not.toHaveBeenCalled(); - if (schemaResult === 'resolve') schema.resolve(); - else schema.reject(new Error('schema also failed')); - await nextTurn(); + let settled = false; + const observed = awaitGraphileBuildReadiness({ schemaResult, addTo, ready, release }) + .catch((error: unknown) => { settled = true; return error; }); + await nextTurn(); // Strict Jest/Node must observe an immediate rejection. + expect(schemaResult).toHaveBeenCalledTimes(1); + if (stage === 'schema') expect(addTo).not.toHaveBeenCalled(); + if (stage !== 'ready') expect(ready).not.toHaveBeenCalled(); expect(release).toHaveBeenCalledTimes(1); - expect(rejected).toBe(false); - + expect(settled).toBe(false); cleanup.resolve(); expect(await observed).toBe(failure); }); - it.each(['resolve', 'reject'] as const)('observes early schema failure while addTo will %s', async (attachmentResult) => { - const schema = deferred(); - const attachment = deferred(); - const schemaFailure = new Error('schema failed'); - const attachmentFailure = new Error('attachment failed'); - const release = jest.fn(); - const observed = awaitGraphileBuildReadiness({ - schemaResult: schema.promise, - addTo: () => attachment.promise, - ready: () => {}, - release, - }).catch((error: unknown) => error); - - schema.reject(schemaFailure); - await nextTurn(); - expect(release).not.toHaveBeenCalled(); - if (attachmentResult === 'resolve') attachment.resolve(); - else attachment.reject(attachmentFailure); - - expect(await observed).toBe(attachmentResult === 'resolve' ? schemaFailure : attachmentFailure); - expect(release).toHaveBeenCalledTimes(1); - }); - - it('observes both schema and Grafserv rejections', async () => { - const schemaFailure = new Error('schema failed'); - const readyFailure = new Error('Grafserv failed'); - const release = jest.fn(); - await expect(awaitGraphileBuildReadiness({ - schemaResult: Promise.reject(schemaFailure), - addTo: () => {}, - ready: () => Promise.reject(readyFailure), - release, - })).rejects.toBe(readyFailure); - await nextTurn(); - expect(release).toHaveBeenCalledTimes(1); - }); - - it('waits for Grafserv to settle before releasing a failed schema', async () => { - const ready = deferred(); - const failure = new Error('schema failed'); - const release = jest.fn(); - const observed = awaitGraphileBuildReadiness({ - schemaResult: Promise.reject(failure), - addTo: () => {}, - ready: () => ready.promise, - release, - }).catch((error: unknown) => error); - - await nextTurn(); - expect(release).not.toHaveBeenCalled(); - ready.resolve(); - expect(await observed).toBe(failure); - expect(release).toHaveBeenCalledTimes(1); - }); - - it.each(['throw', 'reject'] as const)('retains the original error when release will %s', async (mode) => { + it.each(['throw', 'reject'] as const)('retains the original error when cleanup will %s', async (mode) => { const failure = new Error('schema failed'); - const cleanupFailure = new Error('release failed'); + const cleanupFailure = new Error('cleanup failed'); const onReleaseError = jest.fn(); await expect(awaitGraphileBuildReadiness({ - schemaResult: Promise.reject(failure), - addTo: () => {}, - ready: () => {}, - release: () => { - if (mode === 'throw') throw cleanupFailure; - return Promise.reject(cleanupFailure); - }, + schemaResult: () => { throw failure; }, addTo: () => {}, ready: () => {}, + release: () => { if (mode === 'throw') throw cleanupFailure; return Promise.reject(cleanupFailure); }, onReleaseError, })).rejects.toMatchObject({ errors: [failure, cleanupFailure], cause: failure }); expect(onReleaseError).toHaveBeenCalledWith(cleanupFailure); }); - it('includes a reporting failure without replacing the original cause', async () => { + it('includes reporting failure without replacing the original cause', async () => { const failure = new Error('schema failed'); - const cleanupFailure = new Error('release failed'); + const cleanupFailure = new Error('cleanup failed'); const reportingFailure = new Error('reporting failed'); await expect(awaitGraphileBuildReadiness({ - schemaResult: Promise.reject(failure), - addTo: () => {}, - ready: () => {}, + schemaResult: () => { throw failure; }, addTo: () => {}, ready: () => {}, release: () => { throw cleanupFailure; }, onReleaseError: () => { throw reportingFailure; }, })).rejects.toMatchObject({ errors: [failure, cleanupFailure, reportingFailure], cause: failure }); }); + + it('preserves an undefined schema rejection reason', async () => { + await expect(awaitGraphileBuildReadiness({ + schemaResult: () => Promise.reject(undefined), + addTo: () => {}, ready: () => {}, release: () => {}, + })).rejects.toBeUndefined(); + }); }); diff --git a/graphile/graphile-cache/src/__tests__/create-instance.test.ts b/graphile/graphile-cache/src/__tests__/create-instance.test.ts index 1700c159a9..da1f11cb08 100644 --- a/graphile/graphile-cache/src/__tests__/create-instance.test.ts +++ b/graphile/graphile-cache/src/__tests__/create-instance.test.ts @@ -114,7 +114,8 @@ describe('createGraphileInstance', () => { expect(mockPostgraphile).toHaveBeenCalledWith(preset); expect(pgl.getSchemaResult).toHaveBeenCalledTimes(1); expect(mockCreateServer).toHaveBeenCalledWith(handler); - expect(serv.addTo).toHaveBeenCalledWith(handler, httpServer); + expect(pgl.createServ).not.toHaveBeenCalled(); + expect(serv.addTo).not.toHaveBeenCalled(); expect(serv.ready).not.toHaveBeenCalled(); if (first === 'schema') { @@ -123,13 +124,15 @@ describe('createGraphileInstance', () => { attachment.resolve(); } await flushAsyncWork(); - expect(serv.ready).toHaveBeenCalledTimes(first === 'HTTP' ? 1 : 0); + expect(serv.ready).not.toHaveBeenCalled(); + expect(pgl.createServ).toHaveBeenCalledTimes(first === 'schema' ? 1 : 0); expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); expect(settled).not.toHaveBeenCalled(); schema.resolve(schemaResult); attachment.resolve(); await flushAsyncWork(); + expect(serv.addTo).toHaveBeenCalledWith(handler, httpServer); expect(serv.ready).toHaveBeenCalledTimes(1); expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); expect(settled).not.toHaveBeenCalled(); @@ -179,7 +182,7 @@ describe('createGraphileInstance', () => { }); it.each(['throws synchronously', 'rejects immediately'] as const)( - 'observes schema failure that %s while HTTP attachment is pending, then awaits cleanup', + 'observes schema failure that %s before Grafserv construction and awaits cleanup', async (failure) => { const schemaError = new Error('schema failed before HTTP attachment'); const attachment = deferred(); @@ -199,19 +202,13 @@ describe('createGraphileInstance', () => { void instance.then(settled, settled); const rejection = expect(instance).rejects.toBe(schemaError); expect(pgl.getSchemaResult).toHaveBeenCalledTimes(1); - expect(serv.addTo).toHaveBeenCalledTimes(1); - expect(pgl.getSchemaResult.mock.invocationCallOrder[0]).toBeLessThan( - serv.addTo.mock.invocationCallOrder[0], - ); + expect(pgl.createServ).not.toHaveBeenCalled(); + expect(serv.addTo).not.toHaveBeenCalled(); + expect(serv.ready).not.toHaveBeenCalled(); // Cross a macrotask boundary so an unobserved schema rejection fails the test. await flushAsyncWork(); expect(settled).not.toHaveBeenCalled(); - expect(pgl.release).not.toHaveBeenCalled(); - expect(releaseService).not.toHaveBeenCalled(); - - attachment.resolve(); - await flushAsyncWork(); expect(pgl.release).toHaveBeenCalledTimes(1); expect(releaseService).not.toHaveBeenCalled(); expect(settled).not.toHaveBeenCalled(); @@ -227,41 +224,44 @@ describe('createGraphileInstance', () => { }, ); - it('waits for schema settlement and cleanup while preserving the original HTTP failure', async () => { + it('does not start HTTP attachment before schema settles, then preserves an attachment failure', async () => { const schema = deferred(); - const attachment = deferred(); const released = deferred(); const attachmentError = new Error('HTTP attachment failed'); - const schemaError = new Error('schema failed after HTTP attachment'); pgl.getSchemaResult.mockReturnValue(schema.promise); - serv.addTo.mockReturnValue(attachment.promise); + serv.addTo.mockRejectedValue(attachmentError); pgl.release.mockReturnValue(released.promise); - const settled = jest.fn(); const instance = createGraphileInstance({ preset, cacheKey, enableRealtime: true }); - void instance.then(settled, settled); const rejection = expect(instance).rejects.toBe(attachmentError); await flushAsyncWork(); - attachment.reject(attachmentError); - await flushAsyncWork(); + expect(pgl.createServ).not.toHaveBeenCalled(); + expect(serv.addTo).not.toHaveBeenCalled(); expect(pgl.release).not.toHaveBeenCalled(); - expect(releaseService).not.toHaveBeenCalled(); - expect(settled).not.toHaveBeenCalled(); - schema.reject(schemaError); + schema.resolve(schemaResult); await flushAsyncWork(); + expect(pgl.createServ).toHaveBeenCalledTimes(1); + expect(serv.addTo).toHaveBeenCalledTimes(1); expect(pgl.release).toHaveBeenCalledTimes(1); expect(releaseService).not.toHaveBeenCalled(); - expect(settled).not.toHaveBeenCalled(); - + expect(serv.ready).not.toHaveBeenCalled(); released.resolve(); await rejection; expect(releaseService).toHaveBeenCalledTimes(1); - expect(mockError).not.toHaveBeenCalled(); - expect(serv.ready).not.toHaveBeenCalled(); expect(mockCreateRealtimeManager).not.toHaveBeenCalled(); }); + it('cleans up a synchronous createServ failure after schema completes', async () => { + const failure = new Error('createServ failed'); + pgl.createServ.mockImplementation(() => { throw failure; }); + await expect(createGraphileInstance({ preset, cacheKey })).rejects.toBe(failure); + expect(pgl.release).toHaveBeenCalledTimes(1); + expect(releaseService).toHaveBeenCalledTimes(1); + expect(serv.addTo).not.toHaveBeenCalled(); + expect(serv.ready).not.toHaveBeenCalled(); + }); + it('reports cleanup failure with the original build error as its cause', async () => { const buildError = new Error('Grafserv readiness failed'); const releaseError = new Error('PostGraphile release failed'); diff --git a/graphile/graphile-cache/src/__tests__/fixtures/sequential-runtime.cjs b/graphile/graphile-cache/src/__tests__/fixtures/sequential-runtime.cjs new file mode 100644 index 0000000000..9291c91faf --- /dev/null +++ b/graphile/graphile-cache/src/__tests__/fixtures/sequential-runtime.cjs @@ -0,0 +1,157 @@ +// Real locked packages, no database and no rejection-suppression handlers. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const {createRequire} = require('node:module'); +const {createServer} = require('node:http'); +const req = createRequire(path.resolve(__dirname, '../../../package.json')); +const {postgraphile} = req('postgraphile'); +const {PostGraphileAmberPreset} = req('postgraphile/presets/amber'); +const {grafserv} = req('grafserv/express/v4'); +const express = req('express'); +const ts = req('typescript'); +const scenario = process.argv[2]; +const exportsObject = {}; +new Function('exports', ts.transpileModule(fs.readFileSync(path.resolve(__dirname, '../../utils/build-readiness.ts'), 'utf8'), { + compilerOptions: {module: ts.ModuleKind.CommonJS, target: ts.ScriptTarget.ES2022}, +}).outputText)(exportsObject); +const {awaitGraphileBuildReadiness} = exportsObject; +function deferred() { let resolve, reject; const promise = new Promise((res, rej) => {resolve=res;reject=rej;}); return {promise,resolve,reject}; } +const turn = () => new Promise(resolve => setImmediate(resolve)); +async function main() { + const events = []; + const schema = deferred(), attachment = deferred(), config = deferred(), retried = deferred(); + const schemaError = new Error('SCHEMA_SENTINEL'); + const attachmentError = new Error('ATTACHMENT_SENTINEL'); + let attempts = 0; + let schemaBuilds = 0; + let triggerWatch; + let watcherStopped = false; + const watch = scenario.startsWith('watch-'); + const pgl = postgraphile({ + extends: [PostGraphileAmberPreset], pgServices: [], grafserv: {watch}, + schema: {retryOnInitFail: scenario === 'watch-retry-success'}, + plugins: [{ + name: 'SequentialReadinessGate', version: '0.0.0', + gather: { + namespace: 'sequentialReadiness', + initialCache() {events.push('schema:start');return {};}, + async watch(_context, onChange) { + events.push('watch:start'); + triggerWatch = onChange; + if (scenario === 'watch-setup-failure') throw new Error('WATCH_SETUP_SENTINEL'); + return () => {watcherStopped=true;events.push('watch:stop');}; + }, + async main() { + events.push('schema:gather'); + attempts++; + if (scenario === 'watch-retry-success' && attempts > 1) {retried.resolve();return;} + await schema.promise; + }, + }, + schema: { + hooks: { + GraphQLSchema(value) { + schemaBuilds++; + if (scenario === 'watch-schema-failure' || + (scenario === 'watch-schema-recovery' && schemaBuilds === 1)) { + throw new Error('WATCH_SCHEMA_SENTINEL'); + } + return value; + }, + }, + }, + grafserv: { + middleware: { + async setPreset(next) {events.push('config:start');await config.promise;events.push('config:done');return next();}, + }, + }, + }], + }); + assert.deepEqual(events, ['schema:start']); + let serv; + const app=express(), server=createServer(app); + let settled=false; + const result=awaitGraphileBuildReadiness({ + schemaResult: async () => {events.push('schema:get');await pgl.getSchemaResult();events.push('schema:done');}, + addTo: async () => { + events.push('createServ'); + serv=pgl.createServ(grafserv); + events.push('addTo:start'); + await attachment.promise; + await serv.addTo(app,server); + events.push('addTo:done'); + }, + ready: async () => {events.push('ready:start');await serv.ready();events.push('ready:done');}, + release: async () => {events.push('release');await pgl.release();}, + }).then(() => {settled=true;return {success:true};},error => {settled=true;return {error};}); + await turn(); + if (watch) assert(events.includes('watch:start')); + assert(events.includes('schema:get')); + assert(!events.includes('createServ')); + assert(!events.includes('config:start')); + + if (scenario === 'watch-gather-failure' || scenario === 'watch-schema-failure') { + if (scenario === 'watch-gather-failure') schema.reject(schemaError); + else schema.resolve(); + await turn();await turn(); + assert.equal(settled,false); + assert(!events.includes('createServ')); + let laterGetter = 'pending'; + void pgl.getSchemaResult().then(() => {laterGetter='resolved';},error => {laterGetter=error.message;}); + await turn(); + if (scenario === 'watch-gather-failure') { + assert.equal(laterGetter,'PostGraphile instance has been released'); + await assert.rejects(pgl.release(),/has been released/); + assert.equal(watcherStopped,false); + } else { + assert.equal(laterGetter,'pending'); + await pgl.release(); + assert.equal(watcherStopped,true); + } + console.log(JSON.stringify({scenario,pending:true,laterGetter,watcherStopped,events})); + return; + } + if (scenario === 'schema-failure') { + schema.reject(schemaError); + assert.equal((await result).error,schemaError); + assert(!events.includes('createServ')); + assert(!events.includes('ready:start')); + } else { + if (scenario === 'watch-retry-success') { + schema.reject(schemaError); + await retried.promise; + } else schema.resolve(); + await turn(); + if (scenario === 'watch-schema-recovery') { + assert.equal(settled,false); + assert(!events.includes('createServ')); + triggerWatch(); + await turn(); + assert.equal(schemaBuilds,2); + } + assert(events.indexOf('schema:done') < events.indexOf('createServ')); + assert(events.includes('config:start')); + assert(!events.includes('ready:start')); + assert.equal(settled,false); + if (scenario === 'attachment-failure') { + config.resolve(); + attachment.reject(attachmentError); + assert.equal((await result).error,attachmentError); + assert(!events.includes('ready:start')); + } else { + attachment.resolve(); + await turn(); + assert(events.includes('ready:start')); + assert.equal(settled,false); + config.resolve(); + assert.deepEqual(await result,{success:true}); + assert(!events.includes('release')); + await pgl.release(); + if (watch) assert.equal(watcherStopped,true); + } + } + await turn(); + console.log(JSON.stringify({scenario,events})); +} +main().catch(error => {console.error(error);process.exitCode=1;}); diff --git a/graphile/graphile-cache/src/__tests__/sequential-runtime.test.ts b/graphile/graphile-cache/src/__tests__/sequential-runtime.test.ts new file mode 100644 index 0000000000..6ba5987c4b --- /dev/null +++ b/graphile/graphile-cache/src/__tests__/sequential-runtime.test.ts @@ -0,0 +1,35 @@ +import { spawnSync } from 'node:child_process'; +import { join } from 'node:path'; + +const fixture = join(__dirname, 'fixtures/sequential-runtime.cjs'); + +describe('sequential readiness with real locked packages and no database', () => { + it.each(['success', 'schema-failure', 'attachment-failure', 'watch-success', 'watch-retry-success', 'watch-schema-recovery'])( + 'handles %s in strict rejection mode', (scenario) => { + const result = spawnSync(process.execPath, ['--unhandled-rejections=strict', fixture, scenario], { + encoding: 'utf8', timeout: 10_000, + }); + if (result.status !== 0) throw new Error(`${result.error ?? ''}\n${result.stderr}\n${result.stdout}`); + expect(JSON.parse(result.stdout)).toMatchObject({scenario}); + }, + ); + + it.each(['watch-gather-failure', 'watch-schema-failure'])( + 'documents the upstream pending boundary for %s', (scenario) => { + const result = spawnSync(process.execPath, ['--unhandled-rejections=strict', fixture, scenario], { + encoding: 'utf8', timeout: 10_000, + }); + if (result.status !== 0) throw new Error(`${result.error ?? ''}\n${result.stderr}\n${result.stdout}`); + expect(JSON.parse(result.stdout)).toMatchObject({scenario, pending: true}); + }, + ); + + it('detects the upstream watch-startup rejection that readiness cannot observe', () => { + const result = spawnSync(process.execPath, ['--unhandled-rejections=strict', fixture, 'watch-setup-failure'], { + encoding: 'utf8', timeout: 10_000, + }); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(1); + expect(result.stderr).toContain('WATCH_SETUP_SENTINEL'); + }); +}); diff --git a/graphile/graphile-cache/src/create-instance.ts b/graphile/graphile-cache/src/create-instance.ts index 629fbc91f3..aefe73753e 100644 --- a/graphile/graphile-cache/src/create-instance.ts +++ b/graphile/graphile-cache/src/create-instance.ts @@ -48,15 +48,18 @@ export const createGraphileInstance = async ( const { preset, cacheKey, enableRealtime = false } = opts; const pgl = postgraphile(preset); - const serv = pgl.createServ(grafserv); + let serv!: ReturnType; const handler = express(); const httpServer = createServer(handler); await awaitGraphileBuildReadiness({ - schemaResult: (async () => { + schemaResult: async () => { await pgl.getSchemaResult(); - })(), - addTo: () => serv.addTo(handler, httpServer), + }, + addTo: () => { + serv = pgl.createServ(grafserv); + return serv.addTo(handler, httpServer); + }, ready: () => serv.ready(), release: () => releaseFailedBuild(pgl), onReleaseError: (releaseError) => { diff --git a/graphile/graphile-cache/src/utils/build-readiness.ts b/graphile/graphile-cache/src/utils/build-readiness.ts index afd8a4b0a1..f46fffaeb8 100644 --- a/graphile/graphile-cache/src/utils/build-readiness.ts +++ b/graphile/graphile-cache/src/utils/build-readiness.ts @@ -1,32 +1,23 @@ export interface GraphileBuildReadiness { - schemaResult: Promise; + schemaResult(): void | Promise; addTo(): void | PromiseLike; ready(): void | PromiseLike; release(): void | PromiseLike; onReleaseError?(error: unknown): void; } -/** Wait for schema construction, HTTP attachment and Grafserv readiness. */ +/** + * Wait for schema work before starting transport setup. The caller must defer + * Grafserv construction to addTo, so a schema failure leaves no setup to drain. + */ export const awaitGraphileBuildReadiness = async ( build: GraphileBuildReadiness ): Promise => { - // Observe schema rejection immediately, even while addTo is still pending. - // Keeping it as an outcome also lets failure cleanup wait for schema work. - const schemaOutcome = (async () => { - try { - await build.schemaResult; - return { ready: true as const }; - } catch (error: unknown) { - return { ready: false as const, error }; - } - })(); try { + await build.schemaResult(); await build.addTo(); - const [schema] = await Promise.all([schemaOutcome, build.ready()]); - if ('error' in schema) throw schema.error; + await build.ready(); } catch (error) { - // Schema gathering may still be using the services we are about to release. - await schemaOutcome; try { await build.release(); } catch (releaseError) {