Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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';
}
Expand Down
25 changes: 22 additions & 3 deletions graphql/codegen/src/__tests__/codegen/client-generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> {
const { content } = generateOrmClientFile();
const { outputText } = ts.transpileModule(content, {
compilerOptions: {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();

Expand Down
12 changes: 11 additions & 1 deletion graphql/codegen/src/core/codegen/templates/orm-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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';
}
Expand Down
45 changes: 45 additions & 0 deletions graphql/server/src/errors/__tests__/graphql-response.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
30 changes: 30 additions & 0 deletions graphql/server/src/errors/graphql-response.ts
Original file line number Diff line number Diff line change
@@ -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() }],
});
}
23 changes: 10 additions & 13 deletions graphql/server/src/middleware/auth.ts
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -112,26 +115,20 @@ 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;
}

token = result.rows[0];
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 {
Expand Down
17 changes: 5 additions & 12 deletions graphql/server/src/middleware/captcha.ts
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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;
}

Expand Down
12 changes: 12 additions & 0 deletions packages/errors/src/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading