Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error through subscription wrapped in an array #1105

Merged
merged 1 commit into from
Sep 9, 2024
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
21 changes: 10 additions & 11 deletions lib/subscription-connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const { MER_ERR_GQL_SUBSCRIPTION_FORBIDDEN, MER_ERR_GQL_SUBSCRIPTION_UNKNOWN_EXT
const { preSubscriptionParsingHandler, onSubscriptionResolutionHandler, preSubscriptionExecutionHandler, onSubscriptionEndHandler } = require('./handlers')
const { kSubscriptionFactory, kLoaders } = require('./symbols')
const { getProtocolByName } = require('./subscription-protocol')
const { toGraphQLError } = require('./errors')

module.exports = class SubscriptionConnection {
constructor (socket, {
Expand Down Expand Up @@ -63,7 +64,7 @@ module.exports = class SubscriptionConnection {
try {
data = sJSON.parse(isBinary ? message : message.toString())
} catch (e) {
this.sendMessage(this.protocolMessageTypes.GQL_ERROR, null, 'Message must be a JSON string')
this.sendError(new Error('Message must be a JSON string'), null)
return
}

Expand All @@ -78,11 +79,7 @@ module.exports = class SubscriptionConnection {
case this.protocolMessageTypes.GQL_START: {
if (this.isReady) {
this.handleGQLStart(data).catch((e) => {
this.sendMessage(
this.protocolMessageTypes.GQL_ERROR,
id,
e.message
)
this.sendError(e, id)
})
} else {
this.sendMessage(
Expand Down Expand Up @@ -116,11 +113,7 @@ module.exports = class SubscriptionConnection {
break
}

this.sendMessage(
this.protocolMessageTypes.GQL_ERROR,
id,
'Invalid payload type'
)
this.sendError(new Error('Invalid payload type'), id)
}
}

Expand Down Expand Up @@ -389,6 +382,12 @@ module.exports = class SubscriptionConnection {
}
}

sendError (err, id) {
const convertedError = toGraphQLError(err)
// Graphql over websocket spec requires that errors are wrapped in an array
this.sendMessage(this.protocolMessageTypes.GQL_ERROR, id, [convertedError])
}

async handleConnectionInitExtension (extension) {
if (typeof this.onConnect === 'function') {
let authorize = false
Expand Down
14 changes: 7 additions & 7 deletions test/subscription-connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ test('subscription connection sends error message when message is not json strin
t.equal(JSON.stringify({
type: 'error',
id: null,
payload: 'Message must be a JSON string'
payload: [{ message: 'Message must be a JSON string' }]
}), message)
},
protocol: GRAPHQL_TRANSPORT_WS
Expand Down Expand Up @@ -255,7 +255,7 @@ test('subscription connection send error message when GQL_START handler errs', a
t.equal(JSON.stringify({
type: 'error',
id: 1,
payload: 'handleGQLStart error'
payload: [{ message: 'handleGQLStart error' }]
}), message)
},
protocol: GRAPHQL_TRANSPORT_WS
Expand Down Expand Up @@ -284,7 +284,7 @@ test('subscription connection send error message when client message type is inv
t.equal(JSON.stringify({
type: 'error',
id: 1,
payload: 'Invalid payload type'
payload: [{ message: 'Invalid payload type' }]
}), message)
},
protocol: GRAPHQL_TRANSPORT_WS
Expand All @@ -305,7 +305,7 @@ test('subscription connection handles GQL_START message correctly, when payload.
close () {},
send (message) {
t.equal(JSON.stringify(
{ type: 'error', id: 1, payload: 'Must provide document.' }
{ type: 'error', id: 1, payload: [{ message: 'Must provide document.' }] }
), message)
},
protocol: GRAPHQL_TRANSPORT_WS
Expand Down Expand Up @@ -440,7 +440,7 @@ test('subscription connection send GQL_ERROR message if connectionInit extension
t.same(JSON.parse(message), {
id: 1,
type: 'error',
payload: 'Forbidden'
payload: [{ message: 'Forbidden' }]
})
},
protocol: GRAPHQL_TRANSPORT_WS
Expand Down Expand Up @@ -509,7 +509,7 @@ test('subscription connection send GQL_ERROR on unknown extension', async (t) =>
t.same(JSON.parse(message), {
id: 1,
type: 'error',
payload: 'Unknown extension unknown'
payload: [{ message: 'Unknown extension unknown' }]
})
},
protocol: GRAPHQL_TRANSPORT_WS
Expand Down Expand Up @@ -584,7 +584,7 @@ test('subscription connection sends error when trying to execute invalid operati
JSON.stringify({
type: 'error',
id: 1,
payload: 'Invalid operation: query'
payload: [{ message: 'Invalid operation: query' }]
}),
message
)
Expand Down
4 changes: 2 additions & 2 deletions test/subscription-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ test('subscription - should handle preSubscriptionParsing hook errors', async t
t.same(data, {
id: 1,
type: 'error',
payload: 'a preSubscriptionParsing error occurred'
payload: [{ message: 'a preSubscriptionParsing error occurred' }]
})
}
})
Expand Down Expand Up @@ -268,7 +268,7 @@ test('subscription - should handle preSubscriptionExecution hook errors', async
t.same(data, {
id: 1,
type: 'error',
payload: 'a preSubscriptionExecution error occurred'
payload: [{ message: 'a preSubscriptionExecution error occurred' }]
})
}
})
Expand Down
6 changes: 5 additions & 1 deletion test/subscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,11 @@ test('subscription server sends correct error if execution throws', t => {
t.equal(chunk, JSON.stringify({
type: 'error',
id: 1,
payload: 'custom execution error'
payload: [{
message: 'custom execution error',
locations: [{ line: 3, column: 13 }],
path: ['notificationAdded']
}]
}))

client.end()
Expand Down
Loading