Skip to content

Commit

Permalink
fix(runner): always truncate errors (#3063)
Browse files Browse the repository at this point in the history 
# Changes

We can't control what the content of errors, even the one feel like they
are correct might still be too big.

- Fix runner should always truncate errors
- Remove more keys by default
  • Loading branch information
@bodinsamuel
bodinsamuel authored Nov 26, 2024
1 parent 85bd1e9 commit 1d87a74
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 9 deletions.
16 changes: 9 additions & 7 deletions packages/runner/lib/exec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as zod from 'zod';
import * as soap from 'soap';
import * as botbuilder from 'botbuilder';
import tracer from 'dd-trace';
import { errorToObject, metrics } from '@nangohq/utils';
import { errorToObject, metrics, truncateJson } from '@nangohq/utils';
import { logger } from './utils.js';
import type { RunnerOutput } from '@nangohq/types';

Expand Down Expand Up @@ -154,7 +154,9 @@ export async function exec(
success: false,
error: {
type,
payload: Array.isArray(payload) || (typeof payload !== 'object' && payload !== null) ? { message: payload } : payload || {}, // TODO: fix ActionError so payload is always an object
payload: truncateJson(
Array.isArray(payload) || (typeof payload !== 'object' && payload !== null) ? { message: payload } : payload || {}
), // TODO: fix ActionError so payload is always an object
status: 500
},
response: null
Expand All @@ -167,7 +169,7 @@ export async function exec(
success: false,
error: {
type: error.type,
payload: error.payload,
payload: truncateJson(error.payload),
status: error.status
},
response: null
Expand All @@ -180,7 +182,7 @@ export async function exec(
success: false,
error: {
type: 'script_http_error',
payload: typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse,
payload: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse),
status: error.response.status
},
response: null
Expand All @@ -191,7 +193,7 @@ export async function exec(
success: false,
error: {
type: 'script_http_error',
payload: { name: tmp.name || 'Error', code: tmp.code, message: tmp.message },
payload: truncateJson({ name: tmp.name || 'Error', code: tmp.code, message: tmp.message }),
status: 500
},
response: null
Expand All @@ -204,7 +206,7 @@ export async function exec(
success: false,
error: {
type: 'script_internal_error',
payload: { name: tmp.name || 'Error', code: tmp.code, message: tmp.message },
payload: truncateJson({ name: tmp.name || 'Error', code: tmp.code, message: tmp.message }),
status: 500
},
response: null
Expand All @@ -216,7 +218,7 @@ export async function exec(
success: false,
error: {
type: 'script_internal_error',
payload: { name: tmp.name || 'Error', code: tmp.code, message: tmp.message },
payload: truncateJson({ name: tmp.name || 'Error', code: tmp.code, message: tmp.message }),
status: 500
},
response: null
Expand Down
112 changes: 112 additions & 0 deletions packages/runner/lib/exec.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,116 @@ describe('Exec', () => {
});
expect(res.success).toEqual(false);
});

it('should truncate a large error', async () => {
const nangoProps = getNangoProps();
const jsCode = `
fn = async (nango) => {
throw new nango.ActionError({
message: "A manual error",
reason: "a".repeat(1_000_000),
});
};
exports.default = fn
`;
const res = await exec(nangoProps, jsCode);

expect(res.error).toStrictEqual({
payload: {
message: 'A manual error'
},
status: 500,
type: 'action_script_runtime_error'
});
expect(res.success).toEqual(false);
});

it('should redac Authorization', async () => {
const nangoProps = getNangoProps();
const jsCode = `
fn = async (nango) => {
throw new nango.ActionError({
message: "A manual error",
Authorization: 'a very secret secret'
});
};
exports.default = fn
`;
const res = await exec(nangoProps, jsCode);

expect(res.error).toStrictEqual({
payload: {
message: 'A manual error',
Authorization: '[Redacted]'
},
status: 500,
type: 'action_script_runtime_error'
});
expect(res.success).toEqual(false);
});

it('should truncate caught AxiosError', async () => {
const nangoProps = getNangoProps();
const jsCode = `
fn = async (nango) => {
try {
await nango.getConnection();
} catch (err) {
throw new nango.ActionError({
message: "A manual error",
reason: err,
});
}
};
exports.default = fn
`;
const res = await exec(nangoProps, jsCode);

expect(res.error).toStrictEqual({
payload: {
message: 'A manual error',
reason: {
code: expect.any(String),
config: {
adapter: ['xhr', 'http', 'fetch'],
env: {},
headers: {
Accept: 'application/json, text/plain, */*',
'Accept-Encoding': 'gzip, compress, deflate, br',
Authorization: '[Redacted]',
'Content-Type': 'application/json',
'Nango-Is-Dry-Run': 'true',
'Nango-Is-Sync': 'true',
'User-Agent': expect.any(String)
},
maxBodyLength: -1,
maxContentLength: -1,
method: 'get',
params: {
force_refresh: false,
provider_config_key: 'provider-config-key',
refresh_token: false
},
timeout: 0,
transformRequest: [null],
transformResponse: [null],
transitional: {
clarifyTimeoutError: false,
forcedJSONParsing: true,
silentJSONParsing: true
},
url: 'http://localhost:3003/connection/connection-id',
xsrfCookieName: 'XSRF-TOKEN',
xsrfHeaderName: 'X-XSRF-TOKEN'
},
message: expect.any(String),
name: expect.any(String),
status: null
}
},
status: 500,
type: 'action_script_runtime_error'
});
expect(res.success).toEqual(false);
});
});
7 changes: 5 additions & 2 deletions packages/utils/lib/json.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,21 @@ import truncateJsonPkg from 'truncate-json';

export const MAX_LOG_PAYLOAD = 99_000; // in bytes

const ignoredKeys = ['httpAgent', 'httpsAgent', 'trace', '_sessionCache', 'stack'];
/**
* Safely stringify an object (mostly handle circular ref and known problematic keys)
*/
export function stringifyObject(value: any): string {
return safeStringify.stableStringify(
value,
(key, value) => {
if (value instanceof Buffer || key === '_sessionCache') {
if (value instanceof Buffer) {
return '[Buffer]';
}
if (key === 'Authorization') {
return '[Redacted]';
} else if (ignoredKeys.includes(key)) {
return undefined;
}
return value;
},
Expand Down Expand Up @@ -47,7 +50,7 @@ export function stringifyAndTruncateValue(value: any, maxSize: number = MAX_LOG_
* Truncate a JSON
* Will entirely remove properties that are too big
*/
export function truncateJson(value: Record<string, any>, maxSize: number = MAX_LOG_PAYLOAD) {
export function truncateJson<TObject extends Record<string, any>>(value: TObject, maxSize: number = MAX_LOG_PAYLOAD): TObject {
return JSON.parse(truncateJsonPkg(stringifyObject(value), maxSize).jsonString);
}

Expand Down

0 comments on commit 1d87a74

Please sign in to comment.