Skip to content

Commit

Permalink
Error through subscription wrapped in an array (#1105)
Browse files Browse the repository at this point in the history
Co-authored-by: Martin Dubois <[email protected]>
  • Loading branch information
Moonnz and Martin Dubois authored Sep 9, 2024
1 parent 279bc15 commit 4de34c7
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 21 deletions.
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

0 comments on commit 4de34c7

Please sign in to comment.