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

fix(Slack Node): Do not try to parse block if it's already object #9643

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't ideal, but considering the state of type-safety in this package, I think this might be the most realistic option right now .


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
Loading