Skip to content

Commit 0fc9794

Browse files
reconbotreconbot
committed
fix: args that are inline weren't passed to callback functions
eg ```graphql subscription { greetings(name: "Jonas") } ``` Will now work as expected.
1 parent 2bbcab4 commit 0fc9794

11 files changed

+165
-35
lines changed

‎.eslintrc.js

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ module.exports = {
1414
},
1515
plugins: [
1616
'@typescript-eslint',
17+
'mocha-no-only',
1718
],
1819
rules: {
1920
'@typescript-eslint/member-delimiter-style': ['error', {
@@ -32,6 +33,7 @@ module.exports = {
3233
'object-curly-spacing': ['error', 'always'],
3334
'quote-props': ['error', 'as-needed'],
3435
'space-infix-ops': ['error'],
36+
"mocha-no-only/mocha-no-only": ["error"],
3537
indent: ['error', 2],
3638
quotes: ['error', 'single'],
3739
semi: 'off',

‎lib/messages/subscribe-test.ts

+106-22
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,15 @@ describe('messages/subscribe', () => {
8585
assert.deepEqual(state, {
8686
post: [
8787
{ ConnectionId, Data: JSON.stringify({ type: 'connection_ack' }) },
88-
{ ConnectionId, Data: JSON.stringify({ type: 'error', id: 'abcdefg', payload: [{
89-
message: 'Cannot query field "HIHOWEAREYOU" on type "Query".',
90-
locations: [{ line:1, column:3 }],
88+
{
89+
ConnectionId, Data: JSON.stringify({
90+
type: 'error', id: 'abcdefg', payload: [{
91+
message: 'Cannot query field "HIHOWEAREYOU" on type "Query".',
92+
locations: [{ line: 1, column: 3 }],
93+
},
94+
],
95+
}),
9196
},
92-
] }) },
9397
],
9498
delete: [],
9599
})
@@ -102,7 +106,7 @@ describe('messages/subscribe', () => {
102106
const server = await mockServerContext({
103107
apiGatewayManagementApi: {
104108
// eslint-disable-next-line @typescript-eslint/no-empty-function
105-
postToConnection: () => ({ promise: async () => { if(sendErr) { throw new Error('postToConnection Error') } } }),
109+
postToConnection: () => ({ promise: async () => { if (sendErr) { throw new Error('postToConnection Error') } } }),
106110
// eslint-disable-next-line @typescript-eslint/no-empty-function
107111
deleteConnection: () => ({ promise: async () => { } }),
108112
},
@@ -112,7 +116,7 @@ describe('messages/subscribe', () => {
112116
await connection_init({ server, event: connectionInitEvent, message: JSON.parse(connectionInitEvent.body) })
113117
sendErr = true
114118
await subscribe({ server, event, message: JSON.parse(event.body) })
115-
assert.match(error.message, /postToConnection Error/ )
119+
assert.match(error.message, /postToConnection Error/)
116120
})
117121

118122
describe('callbacks', () => {
@@ -132,7 +136,7 @@ describe('messages/subscribe', () => {
132136
hello: () => 'Hello World!',
133137
},
134138
Subscription: {
135-
greetings:{
139+
greetings: {
136140
subscribe: pubsubSubscribe('greetings', {
137141
onSubscribe() {
138142
onSubscribe.push('We did it!')
@@ -146,13 +150,8 @@ describe('messages/subscribe', () => {
146150
},
147151
}
148152

149-
const schema = makeExecutableSchema({
150-
typeDefs,
151-
resolvers,
152-
})
153-
const server = await mockServerContext({
154-
schema,
155-
})
153+
const schema = makeExecutableSchema({ typeDefs, resolvers })
154+
const server = await mockServerContext({ schema })
156155
const event: any = { requestContext: { connectedAt: 1628889984369, connectionId, domainName: 'localhost:3339', eventType: 'MESSAGE', messageDirection: 'IN', messageId: 'el4MNdOJy', requestId: '0yd7bkvXz', requestTimeEpoch: 1628889984774, routeKey: '$default', stage: 'testing' }, isBase64Encoded: false, body: '{"id":"1234","type":"subscribe","payload":{"query":"subscription { greetings }"}}' }
157156

158157
await connection_init({ server, event: connectionInitEvent, message: JSON.parse(connectionInitEvent.body) })
@@ -172,6 +171,96 @@ describe('messages/subscribe', () => {
172171
assert.isEmpty(subscriptions)
173172
})
174173

174+
it('fires onSubscribe with variable args', async () => {
175+
const collectedArgs: any[] = []
176+
177+
const typeDefs = `
178+
type Query {
179+
hello(name: String!): String
180+
}
181+
type Subscription {
182+
greetings(name: String!): String
183+
}
184+
`
185+
const resolvers = {
186+
Query: {
187+
hello: (_, { name }) => `Hello ${name}!`,
188+
},
189+
Subscription: {
190+
greetings: {
191+
subscribe: pubsubSubscribe('greetings', {
192+
onSubscribe(_, args) {
193+
collectedArgs.push(args)
194+
},
195+
}),
196+
resolve: ({ payload }) => {
197+
return payload
198+
},
199+
},
200+
},
201+
}
202+
203+
const schema = makeExecutableSchema({ typeDefs, resolvers })
204+
const server = await mockServerContext({ schema })
205+
const event: any = { requestContext: { connectedAt: 1628889984369, connectionId, domainName: 'localhost:3339', eventType: 'MESSAGE', messageDirection: 'IN', messageId: 'el4MNdOJy', requestId: '0yd7bkvXz', requestTimeEpoch: 1628889984774, routeKey: '$default', stage: 'testing' }, isBase64Encoded: false, body: '{"id":"1234","type":"subscribe","payload":{"query":"subscription($name: String!) { greetings(name: $name) }", "variables":{"name":"Jonas"}}}' }
206+
207+
await connection_init({ server, event: connectionInitEvent, message: JSON.parse(connectionInitEvent.body) })
208+
await subscribe({ server, event, message: JSON.parse(event.body) })
209+
assert.deepEqual(collectedArgs[0], { name: 'Jonas' })
210+
const [subscription] = await collect(server.models.subscription.query({
211+
IndexName: 'ConnectionIndex',
212+
ExpressionAttributeNames: { '#a': 'connectionId' },
213+
ExpressionAttributeValues: { ':1': event.requestContext.connectionId },
214+
KeyConditionExpression: '#a = :1',
215+
}))
216+
assert.containSubset(subscription, { connectionId, subscriptionId: '1234', subscription: JSON.parse(event.body).payload })
217+
})
218+
219+
it('fires onSubscribe with inline args', async () => {
220+
const collectedArgs: any[] = []
221+
222+
const typeDefs = `
223+
type Query {
224+
hello(name: String!): String
225+
}
226+
type Subscription {
227+
greetings(name: String!): String
228+
}
229+
`
230+
const resolvers = {
231+
Query: {
232+
hello: (_, { name }) => `Hello ${name}!`,
233+
},
234+
Subscription: {
235+
greetings: {
236+
subscribe: pubsubSubscribe('greetings', {
237+
onSubscribe(_, args) {
238+
collectedArgs.push(args)
239+
},
240+
}),
241+
resolve: ({ payload }) => {
242+
return payload
243+
},
244+
},
245+
},
246+
}
247+
248+
const schema = makeExecutableSchema({ typeDefs, resolvers })
249+
const server = await mockServerContext({ schema })
250+
const event: any = { requestContext: { connectedAt: 1628889984369, connectionId, domainName: 'localhost:3339', eventType: 'MESSAGE', messageDirection: 'IN', messageId: 'el4MNdOJy', requestId: '0yd7bkvXz', requestTimeEpoch: 1628889984774, routeKey: '$default', stage: 'testing' }, isBase64Encoded: false, body: '{"id":"1234","type":"subscribe","payload":{"query":"subscription { greetings(name: \\"Jonas\\") }"}}' }
251+
252+
await connection_init({ server, event: connectionInitEvent, message: JSON.parse(connectionInitEvent.body) })
253+
await subscribe({ server, event, message: JSON.parse(event.body) })
254+
assert.deepEqual(collectedArgs[0], { name: 'Jonas' })
255+
const [subscription] = await collect(server.models.subscription.query({
256+
IndexName: 'ConnectionIndex',
257+
ExpressionAttributeNames: { '#a': 'connectionId' },
258+
ExpressionAttributeValues: { ':1': event.requestContext.connectionId },
259+
KeyConditionExpression: '#a = :1',
260+
}))
261+
assert.containSubset(subscription, { connectionId, subscriptionId: '1234', subscription: JSON.parse(event.body).payload })
262+
})
263+
175264
it('fires onAfterSubscribe after subscribing', async () => {
176265
const events: string[] = []
177266

@@ -188,7 +277,7 @@ describe('messages/subscribe', () => {
188277
hello: () => 'Hello World!',
189278
},
190279
Subscription: {
191-
greetings:{
280+
greetings: {
192281
subscribe: pubsubSubscribe('greetings', {
193282
onSubscribe() {
194283
events.push('onSubscribe')
@@ -204,13 +293,8 @@ describe('messages/subscribe', () => {
204293
},
205294
}
206295

207-
const schema = makeExecutableSchema({
208-
typeDefs,
209-
resolvers,
210-
})
211-
const server = await mockServerContext({
212-
schema,
213-
})
296+
const schema = makeExecutableSchema({ typeDefs, resolvers })
297+
const server = await mockServerContext({ schema })
214298
const event: any = { requestContext: { connectedAt: 1628889984369, connectionId, domainName: 'localhost:3339', eventType: 'MESSAGE', messageDirection: 'IN', messageId: 'el4MNdOJy', requestId: '0yd7bkvXz', requestTimeEpoch: 1628889984774, routeKey: '$default', stage: 'testing' }, isBase64Encoded: false, body: '{"id":"1234","type":"subscribe","payload":{"query":"subscription { greetings }"}}' }
215299

216300
await connection_init({ server, event: connectionInitEvent, message: JSON.parse(connectionInitEvent.body) })

‎lib/messages/subscribe.ts

+6-5
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ const setupSubscription: MessageHandler<SubscribeMessage> = async ({ server, eve
7070
})
7171
}
7272

73+
const subscriptionId = `${connection.id}|${message.id}`
74+
if (await server.models.subscription.get({ id: subscriptionId })) {
75+
throw new Error(`Subscriber for ${message.id} already exists`)
76+
}
77+
7378
if (execContext.operation.operation !== 'subscription') {
7479
await executeQuery(server, message, contextValue, event)
7580
return
@@ -98,16 +103,12 @@ const setupSubscription: MessageHandler<SubscribeMessage> = async ({ server, eve
98103

99104
const filterData = typeof filter === 'function' ? await filter(root, args, context, info) : filter
100105

101-
const subscriptionId = `${connection.id}|${message.id}`
102106
const subscription: Subscription = {
103107
id: subscriptionId,
104108
topic,
105109
filter: filterData || {},
106110
subscriptionId: message.id,
107-
subscription: {
108-
variableValues: args,
109-
...message.payload,
110-
},
111+
subscription: message.payload,
111112
connectionId: connection.id,
112113
connectionInitPayload: connection.payload,
113114
requestContext: event.requestContext,

‎lib/test/execute-helper.ts

+8-4
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,13 @@ export const executeDoubleQuery = async function* (query: string, {
185185
stayConnected = false,
186186
timeout = 20_000,
187187
id = 1,
188+
skipWaitingForFirstMessage = false,
188189
}: {
189190
url?: string
190191
stayConnected?: boolean
191192
timeout?: number
192193
id?: number
194+
skipWaitingForFirstMessage?: boolean
193195
} = {}): AsyncGenerator<unknown, void, unknown> {
194196
const ws = new WebSocket(url, 'graphql-transport-ws')
195197

@@ -235,11 +237,13 @@ export const executeDoubleQuery = async function* (query: string, {
235237
payload: { query },
236238
})
237239

238-
const firstMessage = await incomingMessages.generator.next()
239-
if (firstMessage.done) {
240-
return
240+
if (!skipWaitingForFirstMessage) {
241+
const firstMessage = await incomingMessages.generator.next()
242+
if (firstMessage.done) {
243+
return
244+
}
245+
yield firstMessage.value
241246
}
242-
yield firstMessage.value
243247

244248
await send({
245249
id: `${id}`,

‎lib/test/graphql-ws-schema.ts

+3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const PORT = 4000
99
const typeDefs = `
1010
type Query {
1111
hello: String
12+
dontResolve: String
1213
}
1314
type Subscription {
1415
greetings: String
@@ -21,6 +22,8 @@ const typeDefs = `
2122
const resolvers = {
2223
Query: {
2324
hello: () => 'Hello World!',
25+
// eslint-disable-next-line @typescript-eslint/no-empty-function
26+
dontResolve: () => new Promise(() => {}),
2427
},
2528
Subscription: {
2629
greetings:{

‎lib/test/integration-events-test.ts

+14-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,20 @@ describe('Events', () => {
6565
await stop()
6666
})
6767

68-
it('errors when duplicating subscription ids', async () => {
68+
// it('errors when duplicating subscription ids with queries', async () => {
69+
// const { url, stop } = await startGqlWSServer()
70+
71+
// const lambdaError = await collect(executeDoubleQuery('{ dontResolve }', { id: 1, skipWaitingForFirstMessage: true }))
72+
// const gqlWSError = await collect(executeDoubleQuery('{ dontResolve }', { id: 1, skipWaitingForFirstMessage: true, url }))
73+
// console.log({ lambdaError, gqlWSError })
74+
// assert.deepEqual(lambdaError[0], gqlWSError[0])
75+
// // This would be exactly equal but apigateway doesn't support close reasons *eye roll*
76+
// assert.containSubset(lambdaError[1], { type: 'close' })
77+
// assert.containSubset(gqlWSError[1], { type: 'close' })
78+
// await stop()
79+
// })
80+
81+
it('errors when duplicating subscription ids with subscriptions', async () => {
6982
const { url, stop } = await startGqlWSServer()
7083

7184
const lambdaError = await collect(executeDoubleQuery('subscription { oneEvent }', { id: 1 }))

‎lib/utils/getResolverAndArgs.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@ import {
55
ExecutionContext,
66
getFieldDef,
77
} from 'graphql/execution/execute'
8+
import { getArgumentValues } from 'graphql/execution/values'
89
import { addPath } from 'graphql/jsutils/Path'
910
import { ServerClosure } from '../types'
1011

11-
type ResolverAndArgs = [ReturnType<typeof getFieldDef>, null, ExecutionContext['variableValues'], ExecutionContext['contextValue'], ReturnType<typeof buildResolveInfo>]
12+
type ResolverAndArgs = [field: ReturnType<typeof getFieldDef>, root: null, args: ExecutionContext['variableValues'], context: ExecutionContext['contextValue'], info: ReturnType<typeof buildResolveInfo>]
1213

1314
export const getResolverAndArgs = (c: Omit<ServerClosure, 'gateway'>) => (execContext: ExecutionContext): ResolverAndArgs => {
1415
// Taken from graphql js - https://github.com/graphql/graphql-js/blob/main/src/subscription/subscribe.js#L190
@@ -40,11 +41,14 @@ export const getResolverAndArgs = (c: Omit<ServerClosure, 'gateway'>) => (execCo
4041
path,
4142
)
4243

44+
const args = getArgumentValues(fieldDef, fieldNodes[0], execContext.variableValues)
45+
const contextValue = execContext.contextValue
46+
4347
return [
4448
fieldDef,
4549
null,
46-
execContext.variableValues,
47-
execContext.contextValue,
50+
args,
51+
contextValue,
4852
info,
4953
]
5054
}

‎mocks/arc-basic-events/lib/graphql.js

+2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const makeManagementAPI = () => {
2525
const typeDefs = `
2626
type Query {
2727
hello: String
28+
dontResolve: String
2829
}
2930
type Subscription {
3031
greetings: String
@@ -41,6 +42,7 @@ const typeDefs = `
4142
const resolvers = {
4243
Query: {
4344
hello: () => 'Hello World!',
45+
dontResolve: () => new Promise(() => {}),
4446
},
4547
Subscription: {
4648
greetings:{

‎package-lock.json

+15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
"esbuild": "^0.12.20",
6464
"esbuild-register": "^3.0.0",
6565
"eslint": "^7.29.0",
66+
"eslint-plugin-mocha-no-only": "^1.1.1",
6667
"graphql": ">= 14.0.0",
6768
"graphql-ws": "^5.3.0",
6869
"inside-out-async": "^1.0.0",

‎rollup.config.js

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export default {
1616
'aws-sdk',
1717
'graphql',
1818
'graphql/execution/execute',
19+
'graphql/execution/values',
1920
// actual deps
2021
'debug',
2122
'streaming-iterables',

0 commit comments

Comments
 (0)