From 4de34c7430e747a9bab032452ddbbf71bccf4fe2 Mon Sep 17 00:00:00 2001 From: Martin Dubois Date: Mon, 9 Sep 2024 09:37:24 +0200 Subject: [PATCH] Error through subscription wrapped in an array (#1105) Co-authored-by: Martin Dubois --- lib/subscription-connection.js | 21 ++++++++++----------- test/subscription-connection.js | 14 +++++++------- test/subscription-hooks.js | 4 ++-- test/subscription.js | 6 +++++- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/lib/subscription-connection.js b/lib/subscription-connection.js index 65d5e1da..4cb4b019 100644 --- a/lib/subscription-connection.js +++ b/lib/subscription-connection.js @@ -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, { @@ -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 } @@ -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( @@ -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) } } @@ -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 diff --git a/test/subscription-connection.js b/test/subscription-connection.js index 26244213..461ec126 100644 --- a/test/subscription-connection.js +++ b/test/subscription-connection.js @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 ) diff --git a/test/subscription-hooks.js b/test/subscription-hooks.js index fc158c85..fe0f87e7 100644 --- a/test/subscription-hooks.js +++ b/test/subscription-hooks.js @@ -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' }] }) } }) @@ -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' }] }) } }) diff --git a/test/subscription.js b/test/subscription.js index 12d1c290..0edc92e6 100644 --- a/test/subscription.js +++ b/test/subscription.js @@ -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()