Skip to content

Commit f08129b

Browse files
glasserglasser
committed
Initial stab at compatibility with newer GraphQL.JS 17 alphas
AS4 is tested against v16 and v17.0.0-alpha.2. They've made it to v17.0.0-alpha.8 by now. We don't build against that alpha for reasons that are fixed by graphql/graphql-js#4425. This commit is based on installing a build of that PR (downloaded from its GH action artifact `npmDist`, unzipped, and `npm pack`'d') and trying to get tests to pass. These tests should pass with v16, v17.0.0-alpha.2, and the #4425 custom build. Incremental delivery tests do NOT pass with the custom build, because the incremental delivery protocol has changed and we haven't updated yet. Smoke test also doesn't pass yet for at least the reason that we haven't asked it to install the right prerelease. Changes include: - Both AS itself and integration tests depend on the particular wording of some error messages, which have changed. (Once we can drop v16 support, AS proper won't need that hack any more, but we're not there yet.) - For some reason we tested the exact size of the JSON serialization of a GraphQL-JS AST in documentStore.test.ts. - One subscriptionCallback debug log line was occurred one tick later with the newer module. I added `await setImmediate` so that it now *consistently* shows up in that slightly later spot.
1 parent 082e4ec commit f08129b

File tree

6 files changed

+76
-57
lines changed

6 files changed

+76
-57
lines changed

‎packages/integration-testsuite/src/apolloServerTests.ts

Lines changed: 51 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,12 @@ export function defineIntegrationTestSuiteApolloServerTests(
360360
});
361361
expect(result.data).toBeUndefined();
362362
expect(result.errors).toBeDefined();
363-
expect(result.errors[0].message).toMatch(
364-
/got invalid value 2; String cannot represent a non string value: 2/,
365-
);
363+
expect([
364+
// graphql v16
365+
`Variable "$x" got invalid value 2; String cannot represent a non string value: 2`,
366+
// graphql v17
367+
'Variable "$x" has invalid value: String cannot represent a non string value: 2',
368+
]).toContain(result.errors[0].message);
366369
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
367370
});
368371

@@ -382,9 +385,12 @@ export function defineIntegrationTestSuiteApolloServerTests(
382385
});
383386
expect(result.data).toBeUndefined();
384387
expect(result.errors).toBeDefined();
385-
expect(result.errors[0].message).toMatch(
388+
expect([
389+
// graphql v16
386390
`Variable "$x" of required type "String!" was not provided.`,
387-
);
391+
// graphql v17
392+
'Variable "$x" has invalid value: Expected a value of non-null type "String!" to be provided.',
393+
]).toContain(result.errors[0].message);
388394
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
389395
});
390396

@@ -404,9 +410,12 @@ export function defineIntegrationTestSuiteApolloServerTests(
404410
});
405411
expect(result.data).toBeUndefined();
406412
expect(result.errors).toBeDefined();
407-
expect(result.errors[0].message).toMatch(
413+
expect([
414+
// graphql v16
408415
`Variable "$x" of required type "[String]!" was not provided.`,
409-
);
416+
// graphql v17
417+
'Variable "$x" has invalid value: Expected a value of non-null type "[String]!" to be provided.',
418+
]).toContain(result.errors[0].message);
410419
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
411420
});
412421

@@ -427,9 +436,12 @@ export function defineIntegrationTestSuiteApolloServerTests(
427436
});
428437
expect(result.data).toBeUndefined();
429438
expect(result.errors).toBeDefined();
430-
expect(result.errors[0].message).toMatch(
439+
expect([
440+
// graphql v16
431441
`Variable "$x" of non-null type "String!" must not be null.`,
432-
);
442+
// graphql v17
443+
'Variable "$x" has invalid value: Expected value of non-null type "String!" not to be null.',
444+
]).toContain(result.errors[0].message);
433445
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
434446
});
435447

@@ -450,9 +462,12 @@ export function defineIntegrationTestSuiteApolloServerTests(
450462
});
451463
expect(result.data).toBeUndefined();
452464
expect(result.errors).toBeDefined();
453-
expect(result.errors[0].message).toMatch(
465+
expect([
466+
// graphql v16
454467
`Variable "$x" of non-null type "[String]!" must not be null.`,
455-
);
468+
// graphql v17
469+
'Variable "$x" has invalid value: Expected value of non-null type "[String]!" not to be null.',
470+
]).toContain(result.errors[0].message);
456471
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
457472
});
458473

@@ -473,10 +488,13 @@ export function defineIntegrationTestSuiteApolloServerTests(
473488
});
474489
expect(result.data).toBeUndefined();
475490
expect(result.errors).toBeDefined();
476-
expect(result.errors[0].message).toBe(
491+
expect([
492+
// graphql v16
477493
`Variable "$x" got invalid value null at "x[0]"; ` +
478494
`Expected non-nullable type "String!" not to be null.`,
479-
);
495+
// graphql v17
496+
'Variable "$x" has invalid value at [0]: Expected value of non-null type "String!" not to be null.',
497+
]).toContain(result.errors[0].message);
480498
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
481499
});
482500

@@ -509,23 +527,17 @@ export function defineIntegrationTestSuiteApolloServerTests(
509527
query: `query ($x:CustomScalar) {hello(x:$x)}`,
510528
variables: { x: 'foo' },
511529
});
512-
expect(result).toMatchInlineSnapshot(`
530+
expect(result.errors).toHaveLength(1);
531+
expect([
532+
// graphql v16
533+
'Variable "$x" got invalid value "foo"; Something bad happened',
534+
// graphql v17
535+
'Variable "$x" has invalid value: Something bad happened',
536+
]).toContain(result.errors[0].message);
537+
expect(result.errors[0].extensions).toMatchInlineSnapshot(`
513538
{
514-
"errors": [
515-
{
516-
"extensions": {
517-
"code": "BAD_USER_INPUT",
518-
"custom": "foo",
519-
},
520-
"locations": [
521-
{
522-
"column": 8,
523-
"line": 1,
524-
},
525-
],
526-
"message": "Variable "$x" got invalid value "foo"; Something bad happened",
527-
},
528-
],
539+
"code": "BAD_USER_INPUT",
540+
"custom": "foo",
529541
}
530542
`);
531543
});
@@ -559,23 +571,17 @@ export function defineIntegrationTestSuiteApolloServerTests(
559571
query: `query ($x:CustomScalar) {hello(x:$x)}`,
560572
variables: { x: 'foo' },
561573
});
562-
expect(result).toMatchInlineSnapshot(`
574+
expect(result.errors).toHaveLength(1);
575+
expect([
576+
// graphql v16
577+
'Variable "$x" got invalid value "foo"; Something bad happened',
578+
// graphql v17
579+
'Variable "$x" has invalid value: Something bad happened',
580+
]).toContain(result.errors[0].message);
581+
expect(result.errors[0].extensions).toMatchInlineSnapshot(`
563582
{
564-
"errors": [
565-
{
566-
"extensions": {
567-
"code": "CUSTOMIZED",
568-
"custom": "foo",
569-
},
570-
"locations": [
571-
{
572-
"column": 8,
573-
"line": 1,
574-
},
575-
],
576-
"message": "Variable "$x" got invalid value "foo"; Something bad happened",
577-
},
578-
],
583+
"code": "CUSTOMIZED",
584+
"custom": "foo",
579585
}
580586
`);
581587
});

‎packages/server/src/__tests__/documentStore.test.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@ describe('ApolloServer documentStore', () => {
5656

5757
await server.executeOperation(operations.simple.op);
5858

59-
expect(
60-
(documentStore as InMemoryLRUCache<DocumentNode>)['cache'].calculatedSize,
61-
).toBe(403);
62-
6359
expect(await documentStore.get(operations.simple.hash)).toMatchObject(
6460
documentNodeMatcher,
6561
);

‎packages/server/src/__tests__/plugin/subscriptionCallback/index.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ describe('SubscriptionCallbackPlugin', () => {
184184
"SubscriptionManager[1234-cats]: \`complete\` request successful",
185185
"SubscriptionManager: Terminating subscriptions for ID: 1234-cats",
186186
"SubscriptionManager: Terminating heartbeat interval for http://mock-router-url.com",
187-
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
188187
"SubscriptionManager[1234-cats]: Subscription completed without errors",
188+
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
189189
]
190190
`);
191191
});
@@ -289,8 +289,8 @@ describe('SubscriptionCallbackPlugin', () => {
289289
"SubscriptionManager[1234-cats]: Sending \`complete\` request to router",
290290
"SubscriptionManager[1234-cats]: \`complete\` request successful",
291291
"SubscriptionManager: Terminating subscriptions for ID: 1234-cats",
292-
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
293292
"SubscriptionManager[1234-cats]: Subscription completed without errors",
293+
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
294294
]
295295
`);
296296
});
@@ -406,8 +406,8 @@ describe('SubscriptionCallbackPlugin', () => {
406406
"SubscriptionManager[1234-cats]: \`complete\` request successful",
407407
"SubscriptionManager: Terminating subscriptions for ID: 1234-cats",
408408
"SubscriptionManager: Terminating heartbeat interval for http://mock-router-url.com",
409-
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
410409
"SubscriptionManager[1234-cats]: Subscription completed without errors",
410+
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
411411
]
412412
`);
413413
});
@@ -524,8 +524,8 @@ describe('SubscriptionCallbackPlugin', () => {
524524
"SubscriptionManager[1234-cats]: \`complete\` request successful",
525525
"SubscriptionManager: Terminating subscriptions for ID: 1234-cats",
526526
"SubscriptionManager: Terminating heartbeat interval for http://mock-router-url.com",
527-
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
528527
"SubscriptionManager[1234-cats]: Subscription completed without errors",
528+
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
529529
]
530530
`);
531531
});
@@ -692,8 +692,8 @@ describe('SubscriptionCallbackPlugin', () => {
692692
"SubscriptionManager[5678-dogs]: \`complete\` request successful",
693693
"SubscriptionManager: Terminating subscriptions for ID: 5678-dogs",
694694
"SubscriptionManager: Terminating heartbeat interval for http://mock-router-url-2.com",
695-
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
696695
"SubscriptionManager[5678-dogs]: Subscription completed without errors",
696+
"SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
697697
]
698698
`);
699699
});

‎packages/server/src/__tests__/runQuery.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,13 @@ it('correctly passes in variables (and arguments)', async () => {
227227

228228
it('throws an error if there are missing variables', async () => {
229229
const query = `query TestVar($base: Int!){ testArgumentValue(base: $base) }`;
230-
const expected = 'Variable "$base" of required type "Int!" was not provided.';
231230
const res = await runQuery({ schema }, { query });
232-
expect(res.errors![0].message).toEqual(expected);
231+
expect([
232+
// graphql 16
233+
'Variable "$base" of required type "Int!" was not provided.',
234+
// graphql 17
235+
'Variable "$base" has invalid value: Expected a value of non-null type "Int!" to be provided.',
236+
]).toContain(res.errors![0].message);
233237
});
234238

235239
it('supports yielding resolver functions', async () => {

‎packages/server/src/plugin/subscriptionCallback/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Logger } from '@apollo/utils.logger';
22
import retry from 'async-retry';
3+
import { setImmediate } from 'node:timers/promises';
34
import { subscribe, type ExecutionResult, type GraphQLError } from 'graphql';
45
import { ensureError, ensureGraphQLError } from '../../errorNormalize.js';
56
import type { ApolloServerPlugin } from '../../externalTypes/index.js';
@@ -185,6 +186,10 @@ export function ApolloServerPluginSubscriptionCallback(
185186
'Server is shutting down. Cleaning up outstanding subscriptions and heartbeat intervals',
186187
);
187188
await subscriptionManager.cleanup();
189+
// This setImmediate makes the precise timing of the log line below
190+
// consistent between when we run our test suite with GraphQL.js v16
191+
// and v17.0.0-alpha.8.
192+
await setImmediate();
188193
logger?.debug(
189194
'Successfully cleaned up outstanding subscriptions and heartbeat intervals.',
190195
);

‎packages/server/src/requestPipeline.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,21 @@ function computeQueryHash(query: string) {
7777

7878
type Mutable<T> = { -readonly [P in keyof T]: T[P] };
7979

80+
// Once GraphQL-JS v17 is released and we make a version of Apollo Server that
81+
// requires it, we can drop this hack, because it lets us break the `execute`
82+
// API into two steps and validate user input explicitly first.
8083
function isBadUserInputGraphQLError(error: GraphQLError): boolean {
8184
return (
8285
error.nodes?.length === 1 &&
8386
error.nodes[0].kind === Kind.VARIABLE_DEFINITION &&
87+
// GraphQL-JS v17 alpha wording
8488
(error.message.startsWith(
85-
`Variable "$${error.nodes[0].variable.name.value}" got invalid value `,
89+
`Variable "$${error.nodes[0].variable.name.value}" has invalid value`,
8690
) ||
91+
// GraphQL-JS v16 wording
92+
error.message.startsWith(
93+
`Variable "$${error.nodes[0].variable.name.value}" got invalid value `,
94+
) ||
8795
error.message.startsWith(
8896
`Variable "$${error.nodes[0].variable.name.value}" of required type `,
8997
) ||

0 commit comments

Comments
 (0)