Skip to content

Commit

Permalink
fix(Slack Node): Do not try to parse block if it's already object (#9643
Browse files Browse the repository at this point in the history
)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
  • Loading branch information
michael-radency and netroy committed Jun 18, 2024
1 parent 1a3f72b commit 8f94dcc
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 2 deletions.
103 changes: 103 additions & 0 deletions packages/core/src/NodeExecuteFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ import type {
WorkflowExecuteMode,
CallbackManager,
INodeParameters,
EnsureTypeOptions,
} from 'n8n-workflow';
import {
ExpressionError,
Expand Down Expand Up @@ -2329,6 +2330,99 @@ export const validateValueAgainstSchema = (
return validationResult.newValue;
};

export function ensureType(
toType: EnsureTypeOptions,
parameterValue: any,
parameterName: string,
errorOptions?: { itemIndex?: number; runIndex?: number; nodeCause?: string },
): string | number | boolean | object {
let returnData = parameterValue;

if (returnData === null) {
throw new ExpressionError(`Parameter '${parameterName}' must not be null`, errorOptions);
}

if (returnData === undefined) {
throw new ExpressionError(
`Parameter '${parameterName}' could not be 'undefined'`,
errorOptions,
);
}

if (['object', 'array', 'json'].includes(toType)) {
if (typeof returnData !== 'object') {
// if value is not an object and is string try to parse it, else throw an error
if (typeof returnData === 'string' && returnData.length) {
try {
const parsedValue = JSON.parse(returnData);
returnData = parsedValue;
} catch (error) {
throw new ExpressionError(`Parameter '${parameterName}' could not be parsed`, {
...errorOptions,
description: error.message,
});
}
} else {
throw new ExpressionError(
`Parameter '${parameterName}' must be an ${toType}, but we got '${String(parameterValue)}'`,
errorOptions,
);
}
} else if (toType === 'json') {
// value is an object, make sure it is valid JSON
try {
JSON.stringify(returnData);
} catch (error) {
throw new ExpressionError(`Parameter '${parameterName}' is not valid JSON`, {
...errorOptions,
description: error.message,
});
}
}

if (toType === 'array' && !Array.isArray(returnData)) {
// value is not an array, but has to be
throw new ExpressionError(
`Parameter '${parameterName}' must be an array, but we got object`,
errorOptions,
);
}
}

try {
if (toType === 'string') {
if (typeof returnData === 'object') {
returnData = JSON.stringify(returnData);
} else {
returnData = String(returnData);
}
}

if (toType === 'number') {
returnData = Number(returnData);
if (Number.isNaN(returnData)) {
throw new ExpressionError(
`Parameter '${parameterName}' must be a number, but we got '${parameterValue}'`,
errorOptions,
);
}
}

if (toType === 'boolean') {
returnData = Boolean(returnData);
}
} catch (error) {
if (error instanceof ExpressionError) throw error;

throw new ExpressionError(`Parameter '${parameterName}' could not be converted to ${toType}`, {
...errorOptions,
description: error.message,
});
}

return returnData;
}

/**
* Returns the requested resolved (all expressions replaced) node parameters.
*
Expand Down Expand Up @@ -2399,6 +2493,15 @@ export function getNodeParameter(
returnData = extractValue(returnData, parameterName, node, nodeType, itemIndex);
}

// Make sure parameter value is the type specified in the ensureType option, if needed convert it
if (options?.ensureType) {
returnData = ensureType(options.ensureType, returnData, parameterName, {
itemIndex,
runIndex,
nodeCause: node.name,
});
}

// Validate parameter value if it has a schema defined(RMC) or validateType defined
returnData = validateValueAgainstSchema(
node,
Expand Down
79 changes: 79 additions & 0 deletions packages/core/test/NodeExecuteFunctions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { SecureContextOptions } from 'tls';
import {
cleanupParameterData,
copyInputItems,
ensureType,
getBinaryDataBuffer,
parseIncomingMessage,
parseRequestObject,
Expand All @@ -25,6 +26,7 @@ import type {
Workflow,
WorkflowHooks,
} from 'n8n-workflow';
import { ExpressionError } from 'n8n-workflow';
import { BinaryDataService } from '@/BinaryData/BinaryData.service';
import nock from 'nock';
import { tmpdir } from 'os';
Expand Down Expand Up @@ -583,4 +585,81 @@ describe('NodeExecuteFunctions', () => {
},
);
});

describe('ensureType', () => {
it('throws error for null value', () => {
expect(() => ensureType('string', null, 'myParam')).toThrowError(
new ExpressionError("Parameter 'myParam' must not be null"),
);
});

it('throws error for undefined value', () => {
expect(() => ensureType('string', undefined, 'myParam')).toThrowError(
new ExpressionError("Parameter 'myParam' could not be 'undefined'"),
);
});

it('returns string value without modification', () => {
const value = 'hello';
const expectedValue = value;
const result = ensureType('string', value, 'myParam');
expect(result).toBe(expectedValue);
});

it('returns number value without modification', () => {
const value = 42;
const expectedValue = value;
const result = ensureType('number', value, 'myParam');
expect(result).toBe(expectedValue);
});

it('returns boolean value without modification', () => {
const value = true;
const expectedValue = value;
const result = ensureType('boolean', value, 'myParam');
expect(result).toBe(expectedValue);
});

it('converts object to string if toType is string', () => {
const value = { name: 'John' };
const expectedValue = JSON.stringify(value);
const result = ensureType('string', value, 'myParam');
expect(result).toBe(expectedValue);
});

it('converts string to number if toType is number', () => {
const value = '10';
const expectedValue = 10;
const result = ensureType('number', value, 'myParam');
expect(result).toBe(expectedValue);
});

it('throws error for invalid conversion to number', () => {
const value = 'invalid';
expect(() => ensureType('number', value, 'myParam')).toThrowError(
new ExpressionError("Parameter 'myParam' must be a number, but we got 'invalid'"),
);
});

it('parses valid JSON string to object if toType is object', () => {
const value = '{"name": "Alice"}';
const expectedValue = JSON.parse(value);
const result = ensureType('object', value, 'myParam');
expect(result).toEqual(expectedValue);
});

it('throws error for invalid JSON string to object conversion', () => {
const value = 'invalid_json';
expect(() => ensureType('object', value, 'myParam')).toThrowError(
new ExpressionError("Parameter 'myParam' could not be parsed"),
);
});

it('throws error for non-array value if toType is array', () => {
const value = { name: 'Alice' };
expect(() => ensureType('array', value, 'myParam')).toThrowError(
new ExpressionError("Parameter 'myParam' must be an array, but we got object"),
);
});
});
});
4 changes: 2 additions & 2 deletions packages/nodes-base/nodes/Slack/V2/GenericFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type {
IWebhookFunctions,
} from 'n8n-workflow';

import { NodeOperationError, jsonParse } from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';

import get from 'lodash/get';

Expand Down Expand Up @@ -162,7 +162,7 @@ export function getMessageContent(
};
break;
case 'block':
content = jsonParse(this.getNodeParameter('blocksUi', i) as string);
content = this.getNodeParameter('blocksUi', i, {}, { ensureType: 'object' }) as IDataObject;

if (includeLinkToWorkflow && Array.isArray(content.blocks)) {
content.blocks.push({
Expand Down
3 changes: 3 additions & 0 deletions packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -622,8 +622,11 @@ export interface IN8nRequestOperationPaginationOffset extends IN8nRequestOperati
};
}

export type EnsureTypeOptions = 'string' | 'number' | 'boolean' | 'object' | 'array' | 'json';
export interface IGetNodeParameterOptions {
contextNode?: INode;
// make sure that returned value would be of specified type, converts it if needed
ensureType?: EnsureTypeOptions;
// extract value from regex, works only when parameter type is resourceLocator
extractValue?: boolean;
// get raw value of parameter with unresolved expressions
Expand Down

0 comments on commit 8f94dcc

Please sign in to comment.