Skip to content

Commit 3357a4f

Browse files
committed
upd corner cases
1 parent ea87df9 commit 3357a4f

3 files changed

Lines changed: 81 additions & 6 deletions

File tree

src/metrics/graphql.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { ApolloServerPlugin, GraphQLRequestContext, GraphQLRequestListener } fro
33
import { GraphQLError } from 'graphql';
44
import HawkCatcher from '@hawk.so/nodejs';
55
import { notifySlowOperation } from './slowOperationAlert';
6-
import { buildGraphqlRequestContext } from './graphqlRequestDetails';
6+
import { buildGraphqlRequestContext, formatGraphqlErrorsForAlert } from './graphqlRequestDetails';
77
/**
88
* GraphQL operation duration histogram
99
* Tracks GraphQL operation duration by operation name and type
@@ -103,7 +103,7 @@ export const graphqlMetricsPlugin: ApolloServerPlugin = {
103103
operationName,
104104
...buildGraphqlRequestContext(ctx),
105105
...(hasErrors && {
106-
errors: ctx.errors!.map((error: GraphQLError) => error.message),
106+
errors: formatGraphqlErrorsForAlert(ctx.errors!),
107107
}),
108108
}
109109
);

src/metrics/graphqlRequestDetails.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { GraphQLRequestContext } from 'apollo-server-plugin-base';
2+
import { GraphQLError } from 'graphql';
23
import { ResolverContextBase } from '../types/graphql';
34
import { truncateText } from './slowOperationAlert';
45

6+
const MAX_ALERT_ERRORS = 10;
7+
const MAX_ALERT_ERRORS_LENGTH = 1200;
8+
59
const SENSITIVE_VARIABLE_KEYS = new Set([
610
'password',
711
'token',
@@ -51,8 +55,15 @@ function sanitizeVariableValue(value: unknown, key: string): unknown {
5155
* @param variables - GraphQL request variables
5256
* @returns sanitized variables
5357
*/
54-
function sanitizeVariables(variables: Record<string, unknown> | undefined): Record<string, unknown> {
55-
if (!variables) {
58+
function sanitizeVariables(
59+
variables: Record<string, unknown> | null | undefined
60+
): Record<string, unknown> {
61+
/**
62+
* Null / non-object values are treated as empty — many clients send
63+
* `variables: null` for operations without variables, and arrays are not a
64+
* valid GraphQL variables map.
65+
*/
66+
if (variables == null || typeof variables !== 'object' || Array.isArray(variables)) {
5667
return {};
5768
}
5869

@@ -97,6 +108,23 @@ function collectHighlightedIds(
97108
return result;
98109
}
99110

111+
/**
112+
* Flatten GraphQL errors into a capped string for Hawk alert context.
113+
* sanitizeContext() only truncates top-level strings, not values nested in arrays.
114+
* Reserves space for an omitted-count suffix and truncateText()'s ellipsis.
115+
*
116+
* @param errors - GraphQL errors from the request
117+
* @returns flattened and truncated error messages
118+
*/
119+
export function formatGraphqlErrorsForAlert(errors: readonly GraphQLError[]): string {
120+
const messages = errors.slice(0, MAX_ALERT_ERRORS).map((error) => error.message);
121+
const omittedCount = errors.length - messages.length;
122+
const omittedSuffix = omittedCount > 0 ? `; …(+${omittedCount} more)` : '';
123+
const maxMessagesLength = Math.max(0, MAX_ALERT_ERRORS_LENGTH - omittedSuffix.length - 1);
124+
125+
return `${truncateText(messages.join('; '), maxMessagesLength)}${omittedSuffix}`;
126+
}
127+
100128
/**
101129
* Build request context for slow GraphQL operation alerts.
102130
*
@@ -105,7 +133,9 @@ function collectHighlightedIds(
105133
*/
106134
export function buildGraphqlRequestContext(ctx: GraphQLRequestContext): Record<string, unknown> {
107135
const context = ctx.context as ResolverContextBase | undefined;
108-
const variables = sanitizeVariables(ctx.request.variables as Record<string, unknown> | undefined);
136+
const variables = sanitizeVariables(
137+
ctx.request.variables as Record<string, unknown> | null | undefined
138+
);
109139
const highlightedIds = collectHighlightedIds(variables);
110140
const alertContext: Record<string, unknown> = {};
111141

test/metrics/graphqlRequestDetails.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { buildGraphqlRequestContext } from '../../src/metrics/graphqlRequestDetails';
1+
import { GraphQLError } from 'graphql';
2+
import {
3+
buildGraphqlRequestContext,
4+
formatGraphqlErrorsForAlert,
5+
} from '../../src/metrics/graphqlRequestDetails';
26

37
describe('buildGraphqlRequestContext', () => {
48
it('should include user, highlighted ids and sanitized variables', () => {
@@ -31,4 +35,45 @@ describe('buildGraphqlRequestContext', () => {
3135
variables: '{"projectId":"6989be3a0bc03531cc430c72","input":{"workspaceId":"workspace-1","password":"[redacted]"},"search":"TypeError"}',
3236
});
3337
});
38+
39+
it('should tolerate null GraphQL variables', () => {
40+
const context = buildGraphqlRequestContext({
41+
context: {
42+
user: {
43+
id: 'user-1',
44+
accessTokenExpired: false,
45+
},
46+
},
47+
request: {
48+
variables: null,
49+
},
50+
} as never);
51+
52+
expect(context).toEqual({
53+
userId: 'user-1',
54+
});
55+
});
56+
});
57+
58+
describe('formatGraphqlErrorsForAlert', () => {
59+
it('should flatten error messages into a single string', () => {
60+
const text = formatGraphqlErrorsForAlert([
61+
new GraphQLError('First error'),
62+
new GraphQLError('Second error'),
63+
]);
64+
65+
expect(text).toBe('First error; Second error');
66+
});
67+
68+
it('should cap the number of errors and truncate long payloads', () => {
69+
const errors = Array.from({ length: 12 }, (_, index) => {
70+
return new GraphQLError(`validation failed on field_${index}: ${'x'.repeat(200)}`);
71+
});
72+
const text = formatGraphqlErrorsForAlert(errors);
73+
74+
expect(text.startsWith('validation failed on field_0:')).toBe(true);
75+
expect(text).toContain('…(+2 more)');
76+
expect(text).toContain('…; …(+2 more)');
77+
expect(text.length).toBeLessThanOrEqual(1200);
78+
});
3479
});

0 commit comments

Comments
 (0)