diff --git a/graphql/codegen/src/__tests__/codegen/__snapshots__/client-generator.test.ts.snap b/graphql/codegen/src/__tests__/codegen/__snapshots__/client-generator.test.ts.snap index f062f5c697..132201a03b 100644 --- a/graphql/codegen/src/__tests__/codegen/__snapshots__/client-generator.test.ts.snap +++ b/graphql/codegen/src/__tests__/codegen/__snapshots__/client-generator.test.ts.snap @@ -245,6 +245,16 @@ export interface OrmClientConfig { realtime?: RealtimeConfig; } +/** + * Describe a single GraphQL error, falling back to its \`extensions.code\` when + * the server omits \`message\` so the error never renders as an empty string. + */ +function describeGraphQLError(error: GraphQLError): string { + if (error.message) return error.message; + const code = error.extensions?.code; + return typeof code === 'string' ? code : 'Unknown error'; +} + /** * Error thrown when GraphQL request fails */ @@ -253,7 +263,7 @@ export class GraphQLRequestError extends Error { public readonly errors: GraphQLError[], public readonly data: unknown = null, ) { - const messages = errors.map((e) => e.message).join('; '); + const messages = errors.map(describeGraphQLError).join('; '); super(\`GraphQL Error: \${messages}\`); this.name = 'GraphQLRequestError'; } diff --git a/graphql/codegen/src/__tests__/codegen/client-generator.test.ts b/graphql/codegen/src/__tests__/codegen/client-generator.test.ts index 0710e520cd..81e0786ce1 100644 --- a/graphql/codegen/src/__tests__/codegen/client-generator.test.ts +++ b/graphql/codegen/src/__tests__/codegen/client-generator.test.ts @@ -59,9 +59,9 @@ type FetchAdapterConstructor = new ( ): Promise<{ ok: boolean; data: T | null; errors?: unknown[] }>; }; -function loadGeneratedFetchAdapter( +function loadGeneratedClientExports( createFetch: () => typeof globalThis.fetch, -): FetchAdapterConstructor { +): Record { const { content } = generateOrmClientFile(); const { outputText } = ts.transpileModule(content, { compilerOptions: { @@ -91,7 +91,14 @@ function loadGeneratedFetchAdapter( { filename: 'generated-orm-client.cjs' }, ); - return mod.exports.FetchAdapter as FetchAdapterConstructor; + return mod.exports; +} + +function loadGeneratedFetchAdapter( + createFetch: () => typeof globalThis.fetch, +): FetchAdapterConstructor { + return loadGeneratedClientExports(createFetch) + .FetchAdapter as FetchAdapterConstructor; } function createThisSensitiveFetch(payload: unknown) { @@ -138,6 +145,18 @@ describe('client-generator', () => { expect(result.content).toContain('GraphQLRequestError'); }); + it('falls back to extensions.code when the server omits message', () => { + const GraphQLRequestError = loadGeneratedClientExports( + () => globalThis.fetch, + ).GraphQLRequestError as new (errors: unknown[]) => Error; + + const error = new GraphQLRequestError([ + { extensions: { code: 'UNAUTHENTICATED' } }, + ]); + + expect(error.message).toBe('GraphQL Error: UNAUTHENTICATED'); + }); + it('exposes an optional fetch injection in OrmClientConfig', () => { const result = generateOrmClientFile(); diff --git a/graphql/codegen/src/core/codegen/templates/orm-client.ts b/graphql/codegen/src/core/codegen/templates/orm-client.ts index d7c5c578fe..f660228a29 100644 --- a/graphql/codegen/src/core/codegen/templates/orm-client.ts +++ b/graphql/codegen/src/core/codegen/templates/orm-client.ts @@ -149,6 +149,16 @@ export interface OrmClientConfig { realtime?: RealtimeConfig; } +/** + * Describe a single GraphQL error, falling back to its `extensions.code` when + * the server omits `message` so the error never renders as an empty string. + */ +function describeGraphQLError(error: GraphQLError): string { + if (error.message) return error.message; + const code = error.extensions?.code; + return typeof code === 'string' ? code : 'Unknown error'; +} + /** * Error thrown when GraphQL request fails */ @@ -157,7 +167,7 @@ export class GraphQLRequestError extends Error { public readonly errors: GraphQLError[], public readonly data: unknown = null, ) { - const messages = errors.map((e) => e.message).join('; '); + const messages = errors.map(describeGraphQLError).join('; '); super(`GraphQL Error: ${messages}`); this.name = 'GraphQLRequestError'; } diff --git a/graphql/server/src/errors/__tests__/graphql-response.test.ts b/graphql/server/src/errors/__tests__/graphql-response.test.ts new file mode 100644 index 0000000000..ef4645afb5 --- /dev/null +++ b/graphql/server/src/errors/__tests__/graphql-response.test.ts @@ -0,0 +1,45 @@ +import { errors } from '@constructive-io/errors'; +import type { Response } from 'express'; + +import { respondWithGraphQLError } from '../graphql-response'; + +const createMockResponse = () => { + const json = jest.fn(); + const res = { status: jest.fn(() => ({ json })) } as unknown as Response; + return { res, json }; +}; + +describe('respondWithGraphQLError', () => { + it('always emits a top-level message so clients never render an empty error', () => { + const { res, json } = createMockResponse(); + + respondWithGraphQLError(res, errors.UNAUTHENTICATED()); + + const [payload] = json.mock.calls[0]; + expect(payload.errors).toHaveLength(1); + expect(payload.errors[0].message).toBe('You must be signed in to do that.'); + expect(payload.errors[0].extensions).toMatchObject({ + code: 'UNAUTHENTICATED', + class: 'public', + http: 401, + }); + }); + + it('responds 200 per the GraphQL-over-HTTP convention', () => { + const { res } = createMockResponse(); + + respondWithGraphQLError(res, errors.CAPTCHA_REQUIRED()); + + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('carries interpolated context in the message for dynamic codes', () => { + const { res, json } = createMockResponse(); + + respondWithGraphQLError(res, errors.INTERNAL_FAILURE({ details: 'boom' })); + + const [payload] = json.mock.calls[0]; + expect(payload.errors[0].message).toContain('boom'); + expect(payload.errors[0].extensions.class).toBe('internal'); + }); +}); diff --git a/graphql/server/src/errors/graphql-response.ts b/graphql/server/src/errors/graphql-response.ts new file mode 100644 index 0000000000..58d72ecd36 --- /dev/null +++ b/graphql/server/src/errors/graphql-response.ts @@ -0,0 +1,30 @@ +/** + * GraphQL-over-HTTP error responses. + * + * Middleware that short-circuits a GraphQL request (auth, captcha, ...) must + * still answer with a valid GraphQL response body. Building that body by hand + * is how a code ends up shipped without a top-level `message`: clients render + * `errors[].message`, so an error carrying only `extensions.code` surfaces as + * an empty string. Route every such response through here so the message and + * extensions both come from the canonical error registry. + * + * @module errors/graphql-response + */ + +import type { ConstructiveError } from '@constructive-io/errors'; +import type { Response } from 'express'; + +/** + * Send a {@link ConstructiveError} as a GraphQL error response. + * + * Uses HTTP 200 per the GraphQL-over-HTTP convention: transport succeeded, the + * operation did not. The error's own `http` hint travels in `extensions`. + */ +export function respondWithGraphQLError( + res: Response, + error: ConstructiveError +): void { + res.status(200).json({ + errors: [{ message: error.message, extensions: error.toExtensions() }], + }); +} diff --git a/graphql/server/src/middleware/auth.ts b/graphql/server/src/middleware/auth.ts index b86e765c89..7e1826a014 100644 --- a/graphql/server/src/middleware/auth.ts +++ b/graphql/server/src/middleware/auth.ts @@ -1,9 +1,12 @@ +import { errors } from '@constructive-io/errors'; import { getNodeEnv } from '@pgpmjs/env'; import { Logger } from '@pgpmjs/logger'; import { PgpmOptions } from '@pgpmjs/types'; import { NextFunction, Request, RequestHandler, Response } from 'express'; import { getPgPool } from 'pg-cache'; import pgQueryContext from 'pg-query-context'; + +import { respondWithGraphQLError } from '../errors/graphql-response'; import './types'; // for Request type const log = new Logger('auth'); @@ -112,9 +115,7 @@ export const createAuthenticateMiddleware = ( if (result?.rowCount === 0) { log.info('[auth] No rows returned, returning UNAUTHENTICATED'); - res.status(200).json({ - errors: [{ extensions: { code: 'UNAUTHENTICATED' } }], - }); + respondWithGraphQLError(res, errors.UNAUTHENTICATED()); return; } @@ -122,16 +123,12 @@ export const createAuthenticateMiddleware = ( log.info(`[auth] Auth success: role=${token.role}, user_id=${token.user_id}`); } catch (e: any) { log.error('[auth] Auth error:', e.message); - res.status(200).json({ - errors: [ - { - extensions: { - code: 'BAD_TOKEN_DEFINITION', - message: e.message, - }, - }, - ], - }); + respondWithGraphQLError( + res, + errors.INTERNAL_FAILURE({ + details: isDev() ? e.message : 'authentication failed', + }) + ); return; } } else { diff --git a/graphql/server/src/middleware/captcha.ts b/graphql/server/src/middleware/captcha.ts index 60f7f90fd0..c6d7c7b1f0 100644 --- a/graphql/server/src/middleware/captcha.ts +++ b/graphql/server/src/middleware/captcha.ts @@ -1,5 +1,8 @@ +import { errors } from '@constructive-io/errors'; import { Logger } from '@pgpmjs/logger'; import type { NextFunction, Request, RequestHandler, Response } from 'express'; + +import { respondWithGraphQLError } from '../errors/graphql-response'; import './types'; // for Request type const log = new Logger('captcha'); @@ -102,23 +105,13 @@ export const createCaptchaMiddleware = (): RequestHandler => { const captchaToken = req.get(CAPTCHA_HEADER); if (!captchaToken) { - res.status(200).json({ - errors: [{ - message: 'CAPTCHA verification required', - extensions: { code: 'CAPTCHA_REQUIRED' }, - }], - }); + respondWithGraphQLError(res, errors.CAPTCHA_REQUIRED()); return; } const valid = await verifyToken(captchaToken, secretKey); if (!valid) { - res.status(200).json({ - errors: [{ - message: 'CAPTCHA verification failed', - extensions: { code: 'CAPTCHA_FAILED' }, - }], - }); + respondWithGraphQLError(res, errors.CAPTCHA_FAILED()); return; } diff --git a/packages/errors/src/registry.ts b/packages/errors/src/registry.ts index bbcff4b27a..7c5cf70fc1 100644 --- a/packages/errors/src/registry.ts +++ b/packages/errors/src/registry.ts @@ -234,6 +234,18 @@ export const registry = { http: 401, message: 'You must be signed in to do that.' }), + CAPTCHA_REQUIRED: defineError({ + code: 'CAPTCHA_REQUIRED', + class: 'public', + http: 400, + message: 'Please complete the CAPTCHA challenge.' + }), + CAPTCHA_FAILED: defineError({ + code: 'CAPTCHA_FAILED', + class: 'public', + http: 403, + message: 'CAPTCHA verification failed. Please try again.' + }), FORBIDDEN: defineError({ code: 'FORBIDDEN', class: 'public',