fix(runner): serialize error correctly (NangoHQ#2567)

bodinsamuel · web-flow · commit 4071ce81497e · 2024-08-01T10:40:40.000Z
## Describe your changes Fixes https://linear.app/nango/issue/NAN-1529/error-serialization-is-broken-in-runnerjobs - Correctly serialize error from runner to jobs I already had fixed this a few weeks ago but the recent migration broke it again. Hopefully, at some point, we'll have some e2e tests. The main idea is to keep looking for known patterns and fallback to std error. I also ejected from `formatScriptError` since dryrun is already too different vs regular exec. At some point we'll need to find a way to use the same code. ### Test ```ts import type { NangoSync } from '../../models'; export default async function runAction(nango: NangoSync): Promise<void> { throw new Error('foobar'); throw new nango.ActionError('foobar'); } ```
diff --git a/packages/jobs/lib/execution/operations/output.ts b/packages/jobs/lib/execution/operations/output.ts
@@ -1,14 +1,15 @@
 import { orchestratorClient } from '../../clients.js';
 import type { JsonValue } from 'type-fest';
 import { logger } from '../../logger.js';
-import { NangoError, formatScriptError } from '@nangohq/shared';
+import { NangoError } from '@nangohq/shared';
 import type { NangoProps } from '@nangohq/shared';
 import { handleSyncError, handleSyncSuccess } from '../sync.js';
 import { handleActionError, handleActionSuccess } from '../action.js';
 import { handleWebhookError, handleWebhookSuccess } from '../webhook.js';
 import { handlePostConnectionError, handlePostConnectionSuccess } from '../postConnection.js';
 import type { ApiError } from '@nangohq/types';
 import type { ClientError } from '@nangohq/nango-orchestrator';
+import { toNangoError } from './utils/errors.js';
 
 export async function handleSuccess({ taskId, nangoProps, output }: { taskId: string; nangoProps: NangoProps; output: JsonValue }): Promise<void> {
     switch (nangoProps.scriptType) {
@@ -45,7 +46,12 @@ export async function handleError({
         status: number;
     };
 }): Promise<void> {
-    const { error: formattedError } = formatScriptError(error.payload, `${nangoProps.scriptType}_script_failure`, nangoProps.syncConfig.sync_name);
+    const formattedError = toNangoError({
+        err: error,
+        defaultErrorType: `${nangoProps.scriptType}_script_failure`,
+        scriptName: nangoProps.syncConfig.sync_name
+    });
+
     switch (nangoProps.scriptType) {
         case 'sync':
             await handleSyncError({ nangoProps, error: formattedError });
@@ -60,6 +66,7 @@ export async function handleError({
             await handlePostConnectionError({ nangoProps, error: formattedError });
             break;
     }
+
     const setFailed = await orchestratorClient.failed({ taskId, error: formattedError });
     if (setFailed.isErr()) {
         await handlePayloadTooBigError({ taskId, error: setFailed.error, nangoProps });
diff --git a/packages/jobs/lib/execution/operations/utils/errors.ts b/packages/jobs/lib/execution/operations/utils/errors.ts
@@ -0,0 +1,27 @@
+import { NangoError, deserializeNangoError } from '@nangohq/shared';
+import { stringifyError } from '@nangohq/utils';
+
+export function toNangoError({ err, defaultErrorType, scriptName }: { err: unknown; defaultErrorType: string; scriptName: string }): NangoError {
+    if (typeof err !== 'object') {
+        return new NangoError('script_invalid_error', err);
+    }
+    if (!err) {
+        return new NangoError('script_invalid_error', null as any);
+    }
+
+    const tmp = deserializeNangoError(err);
+    if (tmp) {
+        return tmp;
+    }
+
+    if ('response' in err && 'data' in (err.response as any)) {
+        const res = err.response as { data: any };
+        return new NangoError(defaultErrorType, res.data);
+    }
+
+    if ('message' in err) {
+        return new NangoError(defaultErrorType, stringifyError(err));
+    }
+
+    return new NangoError(defaultErrorType, `Script for '${scriptName}' failed to execute with unknown error, ${JSON.stringify(err)}`);
+}
diff --git a/packages/jobs/nodemon.json b/packages/jobs/nodemon.json
@@ -1,12 +1,13 @@
 {
     "watch": [
-        "lib",
+        "dist",
         "../shared/dist",
         "../utils/dist",
         "../records/dist",
         "../data-ingestion/dist",
         "../logs/dist",
         "../orchestrator/dist",
+        "../runner/dist",
         "../webhooks/dist",
         "../../.env"
     ],
diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts
@@ -8,7 +8,7 @@ import * as crypto from 'crypto';
 import * as zod from 'zod';
 import * as botbuilder from 'botbuilder';
 import tracer from 'dd-trace';
-import { metrics, stringifyError } from '@nangohq/utils';
+import { errorToObject, metrics } from '@nangohq/utils';
 import { logger } from './utils.js';
 
 export async function exec(
@@ -39,7 +39,7 @@ export async function exec(
 
         try {
             const script = new vm.Script(wrappedCode);
-            const sandbox = {
+            const sandbox: vm.Context = {
                 console,
                 require: (moduleName: string) => {
                     switch (moduleName) {
@@ -56,7 +56,8 @@ export async function exec(
                     }
                 },
                 Buffer,
-                setTimeout
+                setTimeout,
+                Error
             };
 
             const context = vm.createContext(sandbox);
@@ -143,7 +144,9 @@ export async function exec(
                     },
                     response: null
                 };
-            } else if (error instanceof NangoError) {
+            }
+
+            if (error instanceof NangoError) {
                 span.setTag('error', error);
                 return {
                     success: false,
@@ -154,26 +157,51 @@ export async function exec(
                     },
                     response: null
                 };
-            } else {
+            } else if (error instanceof AxiosError) {
                 span.setTag('error', error);
-                if (error instanceof AxiosError && error.response?.data) {
+                if (error.response?.data) {
                     const errorResponse = error.response.data.payload || error.response.data;
                     return {
                         success: false,
                         error: {
-                            type: 'http_error',
+                            type: 'script_http_error',
                             payload: typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse,
                             status: error.response.status
                         },
                         response: null
                     };
+                } else {
+                    const tmp = errorToObject(error);
+                    return {
+                        success: false,
+                        error: {
+                            type: 'script_http_error',
+                            payload: { name: tmp.name || 'Error', code: tmp.code, message: tmp.message },
+                            status: 500
+                        },
+                        response: null
+                    };
                 }
-                span.setTag('error', error);
+            } else if (error instanceof Error) {
+                const tmp = errorToObject(error);
+                span.setTag('error', tmp);
+                return {
+                    success: false,
+                    error: {
+                        type: 'script_internal_error',
+                        payload: { name: tmp.name || 'Error', code: tmp.code, message: tmp.message },
+                        status: 500
+                    },
+                    response: null
+                };
+            } else {
+                const tmp = errorToObject(!error || typeof error !== 'object' ? new Error(JSON.stringify(error)) : error);
+                span.setTag('error', tmp);
                 return {
                     success: false,
                     error: {
-                        type: 'internal_error',
-                        payload: { message: `Error executing code '${stringifyError(error)}'` },
+                        type: 'script_internal_error',
+                        payload: { name: tmp.name || 'Error', code: tmp.code, message: tmp.message },
                         status: 500
                     },
                     response: null
diff --git a/packages/runner/lib/exec.unit.test.ts b/packages/runner/lib/exec.unit.test.ts
@@ -2,34 +2,38 @@ import { expect, describe, it } from 'vitest';
 import { exec } from './exec.js';
 import type { NangoProps, SyncConfig } from '@nangohq/shared';
 
+function getNangoProps(): NangoProps {
+    return {
+        scriptType: 'sync',
+        host: 'http://localhost:3003',
+        connectionId: 'connection-id',
+        environmentId: 1,
+        providerConfigKey: 'provider-config-key',
+        provider: 'provider',
+        activityLogId: '1',
+        secretKey: 'secret-key',
+        nangoConnectionId: 1,
+        syncId: 'sync-id',
+        syncJobId: 1,
+        lastSyncDate: new Date(),
+        dryRun: true,
+        attributes: {},
+        track_deletes: false,
+        logMessages: {
+            counts: { updated: 0, added: 0, deleted: 0 },
+            messages: []
+        },
+        syncConfig: {} as SyncConfig,
+        debug: false,
+        startedAt: new Date(),
+        runnerFlags: {} as any,
+        stubbedMetadata: {}
+    };
+}
+
 describe('Exec', () => {
     it('execute code', async () => {
-        const nangoProps: NangoProps = {
-            scriptType: 'sync',
-            host: 'http://localhost:3003',
-            connectionId: 'connection-id',
-            environmentId: 1,
-            providerConfigKey: 'provider-config-key',
-            provider: 'provider',
-            activityLogId: '1',
-            secretKey: 'secret-key',
-            nangoConnectionId: 1,
-            syncId: 'sync-id',
-            syncJobId: 1,
-            lastSyncDate: new Date(),
-            dryRun: true,
-            attributes: {},
-            track_deletes: false,
-            logMessages: {
-                counts: { updated: 0, added: 0, deleted: 0 },
-                messages: []
-            },
-            syncConfig: {} as SyncConfig,
-            debug: false,
-            startedAt: new Date(),
-            runnerFlags: {} as any,
-            stubbedMetadata: {}
-        };
+        const nangoProps = getNangoProps();
         const jsCode = `
         f = async (nango) => {
             const s = nango.lastSyncDate.toISOString();
@@ -42,4 +46,107 @@ describe('Exec', () => {
         expect(res.error).toEqual(null);
         expect(res.success).toEqual(true);
     });
+
+    it('should return a formatted error when receiving an Error', async () => {
+        const nangoProps = getNangoProps();
+        const jsCode = `
+        fn = async (nango) => {
+            throw new Error('foobar')
+        };
+        exports.default = fn
+        `;
+        const res = await exec(nangoProps, jsCode);
+        expect(res.error).toEqual({
+            payload: {
+                message: 'foobar',
+                name: 'Error'
+            },
+            status: 500,
+            type: 'script_internal_error'
+        });
+        expect(res.success).toEqual(false);
+    });
+
+    it('should return a formatted error when receiving an ActionError', async () => {
+        const nangoProps = getNangoProps();
+        const jsCode = `
+        fn = async (nango) => {
+            throw new nango.ActionError({ message: 'foobar', prop: 'foobar' })
+        };
+        exports.default = fn
+        `;
+        const res = await exec(nangoProps, jsCode);
+        expect(res.error).toEqual({
+            payload: {
+                message: 'foobar',
+                prop: 'foobar'
+            },
+            status: 500,
+            type: 'action_script_runtime_error'
+        });
+        expect(res.success).toEqual(false);
+    });
+
+    it('should return a formatted error when receiving an ActionError with an array', async () => {
+        const nangoProps = getNangoProps();
+        const jsCode = `
+        fn = async (nango) => {
+            throw new nango.ActionError([{id: "foobar"}])
+        };
+        exports.default = fn
+        `;
+        const res = await exec(nangoProps, jsCode);
+        expect(res.error).toEqual({
+            payload: {
+                message: [{ id: 'foobar' }]
+            },
+            status: 500,
+            type: 'action_script_runtime_error'
+        });
+        expect(res.success).toEqual(false);
+    });
+
+    it('should return a formatted error when receiving an invalid error', async () => {
+        const nangoProps = getNangoProps();
+        const jsCode = `
+        fn = async (nango) => {
+            throw new Object({})
+        };
+        exports.default = fn
+        `;
+        const res = await exec(nangoProps, jsCode);
+        expect(res.error).toEqual({
+            payload: {
+                name: 'Error'
+            },
+            status: 500,
+            type: 'script_internal_error'
+        });
+        expect(res.success).toEqual(false);
+    });
+
+    it('should return a formatted error when receiving an AxiosError (without a body)', async () => {
+        const nangoProps = getNangoProps();
+        const jsCode = `
+        fn = async (nango) => {
+            await nango.get({
+                endpoint: '/',
+                baseUrl: 'https://example.dev/'
+            })
+        };
+        exports.default = fn
+        `;
+        const res = await exec(nangoProps, jsCode);
+
+        // NB: it will fail because Nango is not running not because the website is not reachable
+        // NB2: the message is different depending on the system running Node
+        expect(res.error).toMatchObject({
+            payload: {
+                code: 'ECONNREFUSED'
+            },
+            status: 500,
+            type: 'script_http_error'
+        });
+        expect(res.success).toEqual(false);
+    });
 });
diff --git a/packages/runner/nodemon.json b/packages/runner/nodemon.json
@@ -1,5 +1,5 @@
 {
-    "watch": ["lib", "../shared/dist", "../utils/dist", "../../.env"],
+    "watch": ["dist", "../shared/dist", "../utils/dist", "../../.env"],
     "ext": "js,ts,json",
     "ignore": ["lib/**/*.test.ts"],
     "exec": "tsc && tsx -r dotenv/config lib/app.ts dotenv_config_path=./../../.env"
diff --git a/packages/shared/lib/utils/error.ts b/packages/shared/lib/utils/error.ts
@@ -622,6 +622,21 @@ export class NangoError extends Error {
                 this.message = `The sync job results could not be updated: ${this.payload}`;
                 break;
 
+            case 'script_invalid_error':
+                this.status = 500;
+                this.message = `An invalid error of type: ${typeof this.payload}`;
+                break;
+
+            case 'script_http_error':
+                this.status = 500;
+                this.message = `An error occurred during an HTTP call`;
+                break;
+
+            case 'script_internal_error':
+                this.status = 500;
+                this.message = `An internal error occurred during the script execution`;
+                break;
+
             default:
                 this.status = 500;
                 this.type = 'unhandled_' + type;
@@ -645,7 +660,7 @@ export const formatScriptError = (err: any, errorType: string, scriptName: strin
         }
     } else if (err.message) {
         errorMessage = err.message;
-    } else if (typeof err === 'object' && Object.keys(err as object).length > 0) {
+    } else if (err && typeof err === 'object' && Object.keys(err as object).length > 0) {
         errorMessage = stringifyError(err, { pretty: true, stack: true });
     } else {
         errorMessage = String(err);

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`{`
`2`		`- "watch": ["lib", "../shared/dist", "../utils/dist", "../../.env"],`
	`2`	`+ "watch": ["dist", "../shared/dist", "../utils/dist", "../../.env"],`
`3`	`3`	`"ext": "js,ts,json",`
`4`	`4`	`"ignore": ["lib/*/.test.ts"],`
`5`	`5`	`"exec": "tsc && tsx -r dotenv/config lib/app.ts dotenv_config_path=./../../.env"`