fix(graphql-server): never mask errors; same error rules in every environment

graphql/env/README.md

@@ -107,6 +107,9 @@ PostgreSQL extensions or change the API's exposed schemas.
 Each cache limit must be a safe integer of at least `2`. When omitted, Grafast's
 upstream default for that cache remains in effect.
 
+### Grafast Explain
+- `GRAPHILE_EXPLAIN` - Allow clients to request Grafast plan/SQL output via the `x-graphql-explain` header (off unless set)
+
 ### Feature Flags
 - `FEATURES_SIMPLE_INFLECTION` - Enable simple inflection plugin
 - `FEATURES_OPPOSITE_BASE_NAMES` - Enable opposite base names

graphql/env/src/env.ts

@@ -14,6 +14,7 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial
   const scopedIntrospection = getScopedIntrospectionEnv(env);
   const {
     GRAPHILE_SCHEMA,
+    GRAPHILE_EXPLAIN,
     GRAPHILE_QUERY_CACHE_MAX_LENGTH,
     GRAPHILE_OPERATIONS_CACHE_MAX_LENGTH,
     GRAPHILE_OPERATION_PLANS_CACHE_MAX_LENGTH,
@@ -88,6 +89,7 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial
           )
         })
       }),
+      ...(GRAPHILE_EXPLAIN && { explain: parseEnvBoolean(GRAPHILE_EXPLAIN) }),
       ...(GRAPHILE_SCHEMA && {
         schema: GRAPHILE_SCHEMA.includes(',')
           ? GRAPHILE_SCHEMA.split(',').map(s => s.trim())

graphql/server-test/__tests__/upload.integration.test.ts

@@ -224,9 +224,8 @@ const DELETE_APP_BUCKET = `
  * PostgreSQL RLS denials surface in three ways through PostGraphile:
  *   1. An explicit PG error — message contains "permission denied",
  *      "new row violates row-level security", or "No values were".
- *   2. A masked internal error — in production mode PostGraphile masks
- *      PG errors with code INTERNAL_SERVER_ERROR (the raw message is
- *      only logged server-side).
+ *   2. A structured code — `42501` reaches clients as FORBIDDEN (the raw
+ *      message is kept), unrecognized errors as INTERNAL_SERVER_ERROR.
  *   3. The mutation silently affects 0 rows and returns null or an
  *      object with all-null fields (RLS USING clause filtered the row).
  *
@@ -248,6 +247,7 @@ function expectRlsDenied(
         msg.includes('new row violates row-level security') ||
         msg.includes('insufficient_privilege') ||
         msg.includes('No values were') ||
+        code === 'FORBIDDEN' ||
         code === 'INTERNAL_SERVER_ERROR'
     ).toBe(true);
     return;

graphql/server/src/middleware/__tests__/mask-error.test.ts → graphql/server/src/middleware/__tests__/format-error.test.ts

@@ -10,7 +10,7 @@ import {
   validate,
 } from 'graphql';
 
-import { maskError } from '../mask-error';
+import { formatError } from '../format-error';
 
 const ResetPasswordInput = new GraphQLInputObjectType({
   name: 'ResetPasswordInput',
@@ -53,21 +53,11 @@ const run = async (query: string, variables?: Record<string, unknown>) => {
 
   expect(raised.length).toBeGreaterThan(0);
   return raised.map(
-    (error) => maskError(error) as { message: string; extensions?: Record<string, unknown> }
+    (error) => formatError(error) as { message: string; extensions?: Record<string, unknown> }
   );
 };
 
-describe('maskError', () => {
-  const nodeEnv = process.env.NODE_ENV;
-
-  beforeAll(() => {
-    process.env.NODE_ENV = 'production';
-  });
-
-  afterAll(() => {
-    process.env.NODE_ENV = nodeEnv;
-  });
-
+describe('formatError', () => {
   it('surfaces an input field the schema does not define', async () => {
     const [result] = await run('mutation($i: ResetPasswordInput!){ resetPassword(input: $i) }', {
       i: { userId: 'role-1', roleId: 'role-1', newPassword: 'secret' },
@@ -101,17 +91,48 @@ describe('maskError', () => {
       extensions: { code: 'PERSISTED_QUERY_NOT_FOUND' },
     });
 
-    const result = maskError(error) as { message: string; extensions?: Record<string, unknown> };
+    const result = formatError(error) as { message: string; extensions?: Record<string, unknown> };
 
     expect(result.message).toBe('PersistedQueryNotFound');
     expect(result.extensions?.code).toBe('PERSISTED_QUERY_NOT_FOUND');
   });
 
-  it('masks an unrecognized error raised while resolving a field', async () => {
+  it('surfaces an unrecognized resolver error with its real message', async () => {
     const [result] = await run('mutation{ brokenField }');
 
-    expect(result.message).toMatch(/^An unexpected error occurred\. Reference: [0-9a-f]{16}$/);
+    expect(result.message).toBe('relation "internal_secrets" does not exist');
     expect(result.extensions?.code).toBe('INTERNAL_SERVER_ERROR');
-    expect(result.extensions?.errorId).toEqual(expect.any(String));
+    expect(result.extensions?.errorId).toMatch(/^[0-9a-f]{16}$/);
   });
+
+  it('surfaces a permission refusal from postgres as FORBIDDEN', () => {
+    const pgError = Object.assign(new Error('permission denied for table agent_thread'), {
+      code: '42501',
+    });
+    const error = new GraphQLError(pgError.message, { path: ['agentThreads'], originalError: pgError });
+
+    const result = formatError(error) as { message: string; extensions?: Record<string, unknown> };
+
+    expect(result.message).toBe('permission denied for table agent_thread');
+    expect(result.extensions?.code).toBe('FORBIDDEN');
+    expect(result.extensions?.class).toBe('public');
+    expect(result.extensions?.errorId).toBeUndefined();
+  });
+
+  it.each(['production', 'development', 'test', undefined])(
+    'formats errors identically when NODE_ENV is %s',
+    async (env) => {
+      const previous = process.env.NODE_ENV;
+      if (env === undefined) delete process.env.NODE_ENV;
+      else process.env.NODE_ENV = env;
+      try {
+        const [result] = await run('mutation{ brokenField }');
+        expect(result.message).toBe('relation "internal_secrets" does not exist');
+        expect(result.extensions?.code).toBe('INTERNAL_SERVER_ERROR');
+      } finally {
+        if (previous === undefined) delete process.env.NODE_ENV;
+        else process.env.NODE_ENV = previous;
+      }
+    }
+  );
 });

graphql/server/src/middleware/error-handler.ts

@@ -1,6 +1,5 @@
 import './types';
 
-import { getNodeEnv } from '@pgpmjs/env';
 import { Logger } from '@pgpmjs/logger';
 import type { ErrorRequestHandler, NextFunction, Request, Response } from 'express';
 
@@ -10,24 +9,13 @@ import { isApiError } from '../errors/api-errors';
 
 const log = new Logger('error-handler');
 
-const isDevelopment = (): boolean => getNodeEnv() === 'development';
-
 const wantsJson = (req: Request): boolean => {
   const accept = req.get('Accept') || '';
   return accept.includes('application/json')
     || accept.includes('application/graphql-response+json')
     || Boolean(req.is('json'));
 };
 
-const sanitizeMessage = (error: Error): string => {
-  if (isDevelopment()) return error.message;
-  if (isApiError(error)) return error.message;
-  if (error.message?.includes('ECONNREFUSED')) return 'Service temporarily unavailable';
-  if (error.message?.includes('timeout') || error.message?.includes('ETIMEDOUT')) return 'Request timed out';
-  if (error.message?.includes('does not exist')) return 'The requested resource does not exist';
-  return 'An unexpected error occurred';
-};
-
 interface ErrorResponse {
   statusCode: number;
   code: string;
@@ -45,7 +33,7 @@ const categorizeError = (err: Error): ErrorResponse => {
     return {
       statusCode: err.statusCode,
       code: err.code,
-      message: sanitizeMessage(err),
+      message: err.message,
       logLevel: err.statusCode >= 500 ? 'error' : 'warn',
     };
   }
@@ -54,12 +42,12 @@ const categorizeError = (err: Error): ErrorResponse => {
     return { statusCode: 403, code, message: err.message, logLevel: 'warn' };
   }
   if (err.message?.includes('ECONNREFUSED') || err.message?.includes('connection terminated')) {
-    return { statusCode: 503, code: 'SERVICE_UNAVAILABLE', message: sanitizeMessage(err), logLevel: 'error' };
+    return { statusCode: 503, code: 'SERVICE_UNAVAILABLE', message: err.message, logLevel: 'error' };
   }
   if (err.message?.includes('timeout') || err.message?.includes('ETIMEDOUT')) {
-    return { statusCode: 504, code: 'GATEWAY_TIMEOUT', message: sanitizeMessage(err), logLevel: 'error' };
+    return { statusCode: 504, code: 'GATEWAY_TIMEOUT', message: err.message, logLevel: 'error' };
   }
-  return { statusCode: 500, code: 'INTERNAL_ERROR', message: sanitizeMessage(err), logLevel: 'error' };
+  return { statusCode: 500, code: 'INTERNAL_ERROR', message: err.message, logLevel: 'error' };
 };
 
 const sendResponse = (req: Request, res: Response, { statusCode, code, message }: ErrorResponse): void => {
@@ -84,7 +72,7 @@ const logError = (err: Error, req: Request, level: 'warn' | 'error'): void => {
   if (isApiError(err)) {
     log[level]({ event: 'api_error', code: err.code, statusCode: err.statusCode, message: err.message, ...context });
   } else {
-    log[level]({ event: 'unexpected_error', name: err.name, message: err.message, stack: isDevelopment() ? err.stack : undefined, ...context });
+    log[level]({ event: 'unexpected_error', name: err.name, message: err.message, stack: err.stack, ...context });
   }
 };
 

graphql/server/src/middleware/format-error.ts

@@ -0,0 +1,102 @@
+import crypto from 'node:crypto';
+
+import { type ErrorContext, parse } from '@constructive-io/errors';
+import { Logger } from '@pgpmjs/logger';
+import { type GraphQLError, type GraphQLFormattedError } from 'graphql';
+
+const formatErrorLog = new Logger('graphile:formatError');
+
+/**
+ * An error the GraphQL layer raised about the *request*, before any resolver
+ * ran: an unknown input field, a value of the wrong type, a missing required
+ * variable. graphql-js reports variable coercion without an `extensions.code`
+ * (unlike parse/validation, which carry `GRAPHQL_PARSE_FAILED` /
+ * `GRAPHQL_VALIDATION_FAILED`), so code-based classification alone would read
+ * it as a server failure instead of the client's own malformed query.
+ *
+ * A request error is answered before a field is resolved, so it carries no
+ * response `path` — every execution error has one. Coercion wraps the inner
+ * complaint about the value, so `originalError` may be set, but only ever to
+ * another GraphQL-layer error: anything a resolver or the database threw arrives
+ * as a foreign error (a pg error, an `Error`). The wrap
+ * is recognized by name rather than by `instanceof`, because the error is raised
+ * by whichever copy of graphql-js grafast resolved, not by this package's.
+ */
+const isGraphQLLayerError = (value: unknown): boolean =>
+  value == null ||
+  ((value as Error).name === 'GraphQLError' &&
+    isGraphQLLayerError((value as GraphQLError).originalError));
+
+const isRequestError = (error: GraphQLError): boolean =>
+  error.path == null && isGraphQLLayerError((error as { originalError?: unknown }).originalError);
+
+/** The code a request error carries when graphql-js supplied none. */
+const BAD_USER_INPUT = 'BAD_USER_INPUT';
+
+/** The code any other error carries when nothing supplied one. */
+const INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR';
+
+/**
+ * Normalize any GraphQL/database error into a canonical Constructive shape.
+ *
+ * Database errors surface through Grafast without a populated `extensions.code`
+ * (the semantic code lives in the message, and any SQLSTATE/DETAIL lives on the
+ * underlying pg error at `originalError`). We parse `originalError` first so we
+ * can recover the structured code, then fall back to the GraphQL error itself.
+ */
+export const normalizeError = (
+  error: GraphQLError,
+): { code: string | null; context: ErrorContext; class: 'public' | 'internal' } => {
+  const original = (error as { originalError?: unknown }).originalError;
+  const fromOriginal = original ? parse(original) : null;
+  const parsed = fromOriginal?.code ? fromOriginal : parse(error);
+  return { code: parsed.code, context: parsed.context, class: parsed.class };
+};
+
+/**
+ * Format every GraphQL error the same way, in every environment. Nothing is
+ * masked: the client always receives the real message.
+ *
+ * 1. Lift the structured code onto `extensions.code`/`class`/`context` from the
+ *    parsed error, so database errors reach clients with a machine-readable
+ *    code instead of a bare message with empty `extensions`.
+ * 2. A request error graphql-js raised without a code is `BAD_USER_INPUT`.
+ * 3. Any other error without a code is `INTERNAL_SERVER_ERROR`.
+ * 4. Every internal error (unknown, or registered as internal) carries an
+ *    `errorId` and is logged under it, so a report can be matched to the log.
+ */
+export const formatError = (error: GraphQLError): GraphQLFormattedError => {
+  const { code, context, class: errorClass } = normalizeError(error);
+
+  // `extensions` is read-only on GraphQLError, so build a formatted error
+  // rather than mutating it.
+  const extensions: Record<string, unknown> = { ...error.extensions };
+  if (code) {
+    extensions.code = code;
+    extensions.class = errorClass;
+    if (Object.keys(context).length > 0) {
+      extensions.context = context;
+    }
+  }
+
+  let internal = Boolean(code) && errorClass === 'internal';
+  if (!code && !error.extensions?.code) {
+    const requestError = isRequestError(error);
+    extensions.code = requestError ? BAD_USER_INPUT : INTERNAL_SERVER_ERROR;
+    internal = !requestError;
+  }
+
+  if (internal) {
+    const errorId = crypto.randomBytes(8).toString('hex');
+    extensions.errorId = errorId;
+    formatErrorLog.error(`[graphql-error:${errorId}]`, error);
+  }
+
+  // grafserv strips originalError before serializing to the client.
+  return {
+    message: error.message,
+    ...(error.locations ? { locations: error.locations } : {}),
+    ...(error.path ? { path: error.path } : {}),
+    extensions
+  };
+};

graphql/server/src/middleware/graphile.ts

@@ -4,7 +4,6 @@ import { errors } from '@constructive-io/errors';
 import type { ComputeConfig } from '@constructive-io/express-context';
 import { DEFAULT_REQUEST_PROTECTION, protectionPgSettings } from '@constructive-io/express-context';
 import type { ConstructiveOptions } from '@constructive-io/graphql-types';
-import { getNodeEnv } from '@pgpmjs/env';
 import { Logger } from '@pgpmjs/logger';
 import type { NextFunction, Request, RequestHandler, Response } from 'express';
 import { createGraphileInstance, graphileCache,type GraphileCacheEntry } from 'graphile-cache';
@@ -24,12 +23,10 @@ import { AuthCookiePlugin } from '../plugins/auth-cookie-plugin';
 import { createErrorEventsPlugin } from '../plugins/error-events-plugin';
 import { RequestProtectionPlugin } from '../plugins/request-protection-plugin';
 import type { DatabaseSettings } from '../types';
+import { formatError } from './format-error';
 import { makeIntrospectionWiring } from './graphile-introspection';
-import { maskError } from './mask-error';
 import { observeGraphileBuild } from './observability/graphile-build-stats';
 
-const isDev = (): boolean => getNodeEnv() === 'development';
-
 // =============================================================================
 // Single-Flight Pattern: In-Flight Tracking
 // =============================================================================
@@ -131,10 +128,10 @@ const buildPreset = async (
       graphiqlPath: '/graphiql',
       graphiql: true,
       graphiqlOnGraphQLGET: false,
-      maskError
+      maskError: formatError
     },
     grafast: {
-      explain: process.env.NODE_ENV === 'development',
+      explain: graphileOptions?.explain === true,
       context: (requestContext: Partial<Grafast.RequestContext>) => {
       // In grafserv/express/v4, the request is available at requestContext.expressv4.req
         const req = (requestContext as { expressv4?: { req?: Request } })?.expressv4?.req;
@@ -416,7 +413,7 @@ export const graphile = (opts: ConstructiveOptions): RequestHandler => {
         respondWithGraphQLError(
           res,
           errors.INTERNAL_FAILURE({
-            details: isDev() ? e?.message ?? String(e) : 'An unexpected error occurred'
+            details: e?.message ?? String(e)
           })
         );
         return;