From 85f30b27ae282da58a25186d13ff17196dcd7d9c Mon Sep 17 00:00:00 2001 From: Dana <152518854+dana-gill@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:15:49 +0100 Subject: [PATCH] fix(GraphQL Node): Throw error if GraphQL variables are not objects or strings (#11904) --- .../nodes-base/nodes/GraphQL/GraphQL.node.ts | 92 +++++++------- .../nodes/GraphQL/test/GraphQL.node.test.ts | 119 +++++++++++------- .../workflow.error_invalid_expression.json | 32 +++++ .../nodes/GraphQL/test/workflow.json | 57 +++++++-- 4 files changed, 205 insertions(+), 95 deletions(-) create mode 100644 packages/nodes-base/nodes/GraphQL/test/workflow.error_invalid_expression.json diff --git a/packages/nodes-base/nodes/GraphQL/GraphQL.node.ts b/packages/nodes-base/nodes/GraphQL/GraphQL.node.ts index dc9edf2167..07d07011b4 100644 --- a/packages/nodes-base/nodes/GraphQL/GraphQL.node.ts +++ b/packages/nodes-base/nodes/GraphQL/GraphQL.node.ts @@ -418,40 +418,49 @@ export class GraphQL implements INodeType { const gqlQuery = this.getNodeParameter('query', itemIndex, '') as string; if (requestMethod === 'GET') { - if (!requestOptions.qs) { - requestOptions.qs = {}; - } + requestOptions.qs = requestOptions.qs ?? {}; requestOptions.qs.query = gqlQuery; - } else { - if (requestFormat === 'json') { - const jsonBody = { - ...requestOptions.body, - query: gqlQuery, - variables: this.getNodeParameter('variables', itemIndex, {}) as object, - operationName: this.getNodeParameter('operationName', itemIndex) as string, - }; - if (typeof jsonBody.variables === 'string') { - try { - jsonBody.variables = JSON.parse(jsonBody.variables || '{}'); - } catch (error) { - throw new NodeOperationError( - this.getNode(), - 'Using variables failed:\n' + - (jsonBody.variables as string) + - '\n\nWith error message:\n' + - (error as string), - { itemIndex }, - ); - } + } + + if (requestFormat === 'json') { + const variables = this.getNodeParameter('variables', itemIndex, {}); + + let parsedVariables; + if (typeof variables === 'string') { + try { + parsedVariables = JSON.parse(variables || '{}'); + } catch (error) { + throw new NodeOperationError( + this.getNode(), + `Using variables failed:\n${variables}\n\nWith error message:\n${error}`, + { itemIndex }, + ); } - if (jsonBody.operationName === '') { - jsonBody.operationName = null; - } - requestOptions.json = true; - requestOptions.body = jsonBody; + } else if (typeof variables === 'object' && variables !== null) { + parsedVariables = variables; } else { - requestOptions.body = gqlQuery; + throw new NodeOperationError( + this.getNode(), + `Using variables failed:\n${variables}\n\nGraphQL variables should be either an object or a string.`, + { itemIndex }, + ); } + + const jsonBody = { + ...requestOptions.body, + query: gqlQuery, + variables: parsedVariables, + operationName: this.getNodeParameter('operationName', itemIndex) as string, + }; + + if (jsonBody.operationName === '') { + jsonBody.operationName = null; + } + + requestOptions.json = true; + requestOptions.body = jsonBody; + } else { + requestOptions.body = gqlQuery; } let response; @@ -509,22 +518,19 @@ export class GraphQL implements INodeType { throw new NodeApiError(this.getNode(), response.errors as JsonObject, { message }); } } catch (error) { - if (this.continueOnFail()) { - const errorData = this.helpers.returnJsonArray({ - $error: error, - json: this.getInputData(itemIndex), - itemIndex, - }); - const exectionErrorWithMetaData = this.helpers.constructExecutionMetaData(errorData, { - itemData: { item: itemIndex }, - }); - returnItems.push(...exectionErrorWithMetaData); - continue; + if (!this.continueOnFail()) { + throw error; } - throw error; + + const errorData = this.helpers.returnJsonArray({ + error: error.message, + }); + const exectionErrorWithMetaData = this.helpers.constructExecutionMetaData(errorData, { + itemData: { item: itemIndex }, + }); + returnItems.push(...exectionErrorWithMetaData); } } - return [returnItems]; } } diff --git a/packages/nodes-base/nodes/GraphQL/test/GraphQL.node.test.ts b/packages/nodes-base/nodes/GraphQL/test/GraphQL.node.test.ts index 4d099f5769..2b6014fcd4 100644 --- a/packages/nodes-base/nodes/GraphQL/test/GraphQL.node.test.ts +++ b/packages/nodes-base/nodes/GraphQL/test/GraphQL.node.test.ts @@ -1,54 +1,83 @@ -import type { WorkflowTestData } from '@test/nodes/types'; -import { executeWorkflow } from '@test/nodes/ExecuteWorkflow'; -import * as Helpers from '@test/nodes/Helpers'; +/* eslint-disable n8n-nodes-base/node-filename-against-convention */ +import nock from 'nock'; + +import { + equalityTest, + getWorkflowFilenames, + initBinaryDataService, + setup, + workflowToTests, +} from '@test/nodes/Helpers'; describe('GraphQL Node', () => { - const mockResponse = { - data: { - nodes: {}, - }, - }; + const workflows = getWorkflowFilenames(__dirname); + const workflowTests = workflowToTests(workflows); - const tests: WorkflowTestData[] = [ - { - description: 'should run Request Format JSON', - input: { - workflowData: Helpers.readJsonFileSync('nodes/GraphQL/test/workflow.json'), - }, - output: { - nodeExecutionOrder: ['Start'], - nodeData: { - 'Fetch Request Format JSON': [ - [ + const baseUrl = 'https://api.n8n.io/'; + + beforeAll(async () => { + await initBinaryDataService(); + nock.disableNetConnect(); + + nock(baseUrl) + .matchHeader('accept', 'application/json') + .matchHeader('content-type', 'application/json') + .matchHeader('user-agent', 'axios/1.7.4') + .matchHeader('content-length', '263') + .matchHeader('accept-encoding', 'gzip, compress, deflate, br') + .post( + '/graphql', + '{"query":"query {\\n nodes(pagination: { limit: 1 }) {\\n data {\\n id\\n attributes {\\n name\\n displayName\\n description\\n group\\n codex\\n createdAt\\n }\\n }\\n }\\n}","variables":{},"operationName":null}', + ) + .reply(200, { + data: { + nodes: { + data: [ { - json: mockResponse, + id: '1', + attributes: { + name: 'n8n-nodes-base.activeCampaign', + displayName: 'ActiveCampaign', + description: 'Create and edit data in ActiveCampaign', + group: '["transform"]', + + codex: { + data: { + details: + 'ActiveCampaign is a cloud software platform that allows customer experience automation, which combines email marketing, marketing automation, sales automation, and CRM categories. Use this node when you want to interact with your ActiveCampaign account.', + resources: { + primaryDocumentation: [ + { + url: 'https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-base.activecampaign/', + }, + ], + credentialDocumentation: [ + { + url: 'https://docs.n8n.io/integrations/builtin/credentials/activeCampaign/', + }, + ], + }, + categories: ['Marketing'], + nodeVersion: '1.0', + codexVersion: '1.0', + }, + }, + createdAt: '2019-08-30T22:54:39.934Z', + }, }, ], - ], - }, - }, - nock: { - baseUrl: 'https://api.n8n.io', - mocks: [ - { - method: 'post', - path: '/graphql', - statusCode: 200, - responseBody: mockResponse, }, - ], - }, - }, - ]; - - const nodeTypes = Helpers.setup(tests); - - test.each(tests)('$description', async (testData) => { - const { result } = await executeWorkflow(testData, nodeTypes); - const resultNodeData = Helpers.getResultNodeData(result, testData); - resultNodeData.forEach(({ nodeName, resultData }) => - expect(resultData).toEqual(testData.output.nodeData[nodeName]), - ); - expect(result.finished).toEqual(true); + }, + }); }); + + afterAll(() => { + nock.restore(); + }); + + const nodeTypes = setup(workflowTests); + + for (const workflow of workflowTests) { + test(workflow.description, async () => await equalityTest(workflow, nodeTypes)); + } }); diff --git a/packages/nodes-base/nodes/GraphQL/test/workflow.error_invalid_expression.json b/packages/nodes-base/nodes/GraphQL/test/workflow.error_invalid_expression.json new file mode 100644 index 0000000000..dd01638b68 --- /dev/null +++ b/packages/nodes-base/nodes/GraphQL/test/workflow.error_invalid_expression.json @@ -0,0 +1,32 @@ +{ + "meta": { + "templateId": "216", + "instanceId": "ee90fdf8d57662f949e6c691dc07fa0fd2f66e1eee28ed82ef06658223e67255" + }, + "nodes": [ + { + "parameters": { + "endpoint": "https://graphql-teas-endpoint.netlify.app/", + "requestFormat": "json", + "query": "query getAllTeas($name: String) {\n teas(name: $name) {\n name,\n id\n }\n}", + "variables": "={{ 1 }}" + }, + "id": "7aece03f-e0d9-4f49-832c-fc6465613ca7", + "name": "Test: Errors on unsuccessful Expression validation", + "type": "n8n-nodes-base.graphql", + "typeVersion": 1, + "position": [660, 200], + "onError": "continueRegularOutput" + } + ], + "connections": {}, + "pinData": { + "Test: Errors on unsuccessful Expression validation": [ + { + "json": { + "error": "Using variables failed:\n1\n\nGraphQL variables should be either an object or a string." + } + } + ] + } +} diff --git a/packages/nodes-base/nodes/GraphQL/test/workflow.json b/packages/nodes-base/nodes/GraphQL/test/workflow.json index 690171afc5..78bb20c5bf 100644 --- a/packages/nodes-base/nodes/GraphQL/test/workflow.json +++ b/packages/nodes-base/nodes/GraphQL/test/workflow.json @@ -1,16 +1,16 @@ { "meta": { - "templateCredsSetupCompleted": true, - "instanceId": "104a4d08d8897b8bdeb38aaca515021075e0bd8544c983c2bb8c86e6a8e6081c" + "templateId": "216", + "instanceId": "ee90fdf8d57662f949e6c691dc07fa0fd2f66e1eee28ed82ef06658223e67255" }, "nodes": [ { "parameters": {}, - "id": "fb826323-2e48-4f11-bb0e-e12de32e22ee", + "id": "5e2ef15b-2c6c-412f-a9da-515b5211386e", "name": "When clicking ‘Test workflow’", "type": "n8n-nodes-base.manualTrigger", "typeVersion": 1, - "position": [180, 160] + "position": [420, 100] }, { "parameters": { @@ -21,8 +21,8 @@ "name": "Fetch Request Format JSON", "type": "n8n-nodes-base.graphql", "typeVersion": 1, - "position": [420, 160], - "id": "7f8ceaf4-b82f-48d5-be0b-9fe3bfb35ee4" + "position": [700, 140], + "id": "e1c750a0-8d6c-4e81-8111-3218e1e6e69f" } ], "connections": { @@ -38,5 +38,48 @@ ] } }, - "pinData": {} + "pinData": { + "Fetch Request Format JSON": [ + { + "json": { + "data": { + "nodes": { + "data": [ + { + "id": "1", + "attributes": { + "name": "n8n-nodes-base.activeCampaign", + "displayName": "ActiveCampaign", + "description": "Create and edit data in ActiveCampaign", + "group": "[\"transform\"]", + "codex": { + "data": { + "details": "ActiveCampaign is a cloud software platform that allows customer experience automation, which combines email marketing, marketing automation, sales automation, and CRM categories. Use this node when you want to interact with your ActiveCampaign account.", + "resources": { + "primaryDocumentation": [ + { + "url": "https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-base.activecampaign/" + } + ], + "credentialDocumentation": [ + { + "url": "https://docs.n8n.io/integrations/builtin/credentials/activeCampaign/" + } + ] + }, + "categories": ["Marketing"], + "nodeVersion": "1.0", + "codexVersion": "1.0" + } + }, + "createdAt": "2019-08-30T22:54:39.934Z" + } + } + ] + } + } + } + } + ] + } }