From e14fb4a61d68a23a6383774331e8a2511d423b85 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Wed, 8 Jan 2025 13:58:25 -0500 Subject: [PATCH 01/19] Tweak runner output error type --- packages/types/lib/runner/index.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/types/lib/runner/index.ts b/packages/types/lib/runner/index.ts index 9399849337c..8affd66894a 100644 --- a/packages/types/lib/runner/index.ts +++ b/packages/types/lib/runner/index.ts @@ -2,6 +2,11 @@ export interface RunnerOutputError { type: string; payload: Record; status: number; + upstream_response?: { + status: number; + headers: Record; + body: Record; + }; } export interface RunnerOutput { success: boolean; From 979ac051e1553752bc4b510c2309c2baa5192dc5 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Wed, 8 Jan 2025 14:46:05 -0500 Subject: [PATCH 02/19] Add specialized handling for http errors in exec --- packages/runner/lib/exec.ts | 17 ++++++-- packages/runner/lib/exec.unit.test.ts | 57 ++++++++++++++++++++++++--- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index 3717adafa67..754e7f90159 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -1,5 +1,5 @@ import type { NangoProps } from '@nangohq/shared'; -import { AxiosError } from 'axios'; +import { isAxiosError } from 'axios'; import { ActionError, NangoSync, NangoAction, instrumentSDK, SpanTypes, validateData, NangoError } from '@nangohq/shared'; import { Buffer } from 'buffer'; import * as vm from 'node:vm'; @@ -174,7 +174,7 @@ export async function exec( }, response: null }; - } else if (err instanceof AxiosError) { + } else if (isAxiosError(err)) { span.setTag('error', err); if (err.response?.data) { const errorResponse = err.response.data.payload || err.response.data; @@ -183,7 +183,16 @@ export async function exec( error: { type: 'script_http_error', payload: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse), - status: err.response.status + status: err.response.status, + upstream_response: { + status: err.response.status, + headers: Object.fromEntries( + Object.entries(err.response.headers) + .map(([k, v]) => [k.toLowerCase(), v]) + .filter(([k]) => k === 'content-type' || k.startsWith('x-ratelimit')) + ), + body: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse) + } }, response: null }; @@ -192,7 +201,7 @@ export async function exec( return { success: false, error: { - type: 'script_http_error', + type: 'script_network_error', payload: truncateJson({ name: tmp.name || 'Error', code: tmp.code, message: tmp.message }), status: 500 }, diff --git a/packages/runner/lib/exec.unit.test.ts b/packages/runner/lib/exec.unit.test.ts index 508a9e92927..630169ea027 100644 --- a/packages/runner/lib/exec.unit.test.ts +++ b/packages/runner/lib/exec.unit.test.ts @@ -126,14 +126,15 @@ describe('Exec', () => { expect(res.success).toEqual(false); }); - it('should return a formatted error when receiving an AxiosError (without a body)', async () => { + it('should return a script_network_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/' - }) + const err = new Error("Something broke"); + err.isAxiosError = true; + err.code = "ECONNREFUSED"; + + throw err; }; exports.default = fn `; @@ -146,6 +147,50 @@ describe('Exec', () => { code: 'ECONNREFUSED' }, status: 500, + type: 'script_network_error' + }); + expect(res.success).toEqual(false); + }); + + it('should return a script_network_error when receiving an AxiosError (without a body)', async () => { + const nangoProps = getNangoProps(); + const jsCode = ` + fn = async (nango) => { + const err = new Error("Something broke"); + err.isAxiosError = true; + err.code = "ERR_BAD_RESPONSE"; + err.response = { + status: 404, + headers: { + 'Content-Type': 'application/json', + 'Content-Security-Policy': 'blech', + 'X-RateLimit-Limit': '100', + }, + data: { error: "Not found" } + } + throw err; + }; + 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).toEqual({ + payload: { + error: 'Not found' + }, + status: 404, + upstream_response: { + body: { + error: 'Not found' + }, + headers: { + 'content-type': 'application/json', + 'x-ratelimit-limit': '100' + }, + status: 404 + }, type: 'script_http_error' }); expect(res.success).toEqual(false); @@ -174,7 +219,7 @@ describe('Exec', () => { expect(res.success).toEqual(false); }); - it('should redac Authorization', async () => { + it('should redact Authorization', async () => { const nangoProps = getNangoProps(); const jsCode = ` fn = async (nango) => { From e9933d30fe18304d1fc8a68a46663d27b8c817b2 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Wed, 8 Jan 2025 14:46:31 -0500 Subject: [PATCH 03/19] Fix ratelimit header filter --- packages/runner/lib/exec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index 754e7f90159..33f837cd8c1 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -189,7 +189,7 @@ export async function exec( headers: Object.fromEntries( Object.entries(err.response.headers) .map(([k, v]) => [k.toLowerCase(), v]) - .filter(([k]) => k === 'content-type' || k.startsWith('x-ratelimit')) + .filter(([k]) => k === 'content-type' || k.startsWith('x-rate')) ), body: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse) } From 8045a2d4482b97d65984d7ebe6b526458f527397 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Wed, 8 Jan 2025 15:03:44 -0500 Subject: [PATCH 04/19] Cleanup --- packages/runner/lib/exec.ts | 12 +++++++----- packages/runner/lib/exec.unit.test.ts | 7 ++++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index 33f837cd8c1..15a60702a5a 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -178,6 +178,12 @@ export async function exec( span.setTag('error', err); if (err.response?.data) { const errorResponse = err.response.data.payload || err.response.data; + const headers = Object.fromEntries( + Object.entries(err.response.headers) + .map(([k, v]) => [k.toLowerCase(), v]) + .filter(([k]) => k === 'content-type' || k.startsWith('x-rate')) + ); + return { success: false, error: { @@ -186,11 +192,7 @@ export async function exec( status: err.response.status, upstream_response: { status: err.response.status, - headers: Object.fromEntries( - Object.entries(err.response.headers) - .map(([k, v]) => [k.toLowerCase(), v]) - .filter(([k]) => k === 'content-type' || k.startsWith('x-rate')) - ), + headers, body: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse) } }, diff --git a/packages/runner/lib/exec.unit.test.ts b/packages/runner/lib/exec.unit.test.ts index 630169ea027..da05244bd1a 100644 --- a/packages/runner/lib/exec.unit.test.ts +++ b/packages/runner/lib/exec.unit.test.ts @@ -26,7 +26,12 @@ function getNangoProps(): NangoProps { syncConfig: {} as SyncConfig, debug: false, startedAt: new Date(), - runnerFlags: {} as any, + runnerFlags: { + validateActionInput: false, + validateActionOutput: false, + validateSyncMetadata: false, + validateSyncRecords: false + }, stubbedMetadata: {}, endUser: null }; From 5ec18b9d40a176f8826c6413bc5ea4bd14ec47b6 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Wed, 8 Jan 2025 16:35:45 -0500 Subject: [PATCH 05/19] Go back to instanceof check --- packages/runner/lib/exec.ts | 7 ++++--- packages/types/lib/runner/index.ts | 13 ++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index 15a60702a5a..e11fc7f4b67 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -1,5 +1,5 @@ import type { NangoProps } from '@nangohq/shared'; -import { isAxiosError } from 'axios'; +import { AxiosError } from 'axios'; import { ActionError, NangoSync, NangoAction, instrumentSDK, SpanTypes, validateData, NangoError } from '@nangohq/shared'; import { Buffer } from 'buffer'; import * as vm from 'node:vm'; @@ -174,13 +174,14 @@ export async function exec( }, response: null }; - } else if (isAxiosError(err)) { + } else if (err instanceof AxiosError) { span.setTag('error', err); if (err.response?.data) { const errorResponse = err.response.data.payload || err.response.data; + const headers = Object.fromEntries( Object.entries(err.response.headers) - .map(([k, v]) => [k.toLowerCase(), v]) + .map(([k, v]) => [k.toLowerCase(), v.toString()]) .filter(([k]) => k === 'content-type' || k.startsWith('x-rate')) ); diff --git a/packages/types/lib/runner/index.ts b/packages/types/lib/runner/index.ts index 8affd66894a..be79dc0e79f 100644 --- a/packages/types/lib/runner/index.ts +++ b/packages/types/lib/runner/index.ts @@ -2,12 +2,15 @@ export interface RunnerOutputError { type: string; payload: Record; status: number; - upstream_response?: { - status: number; - headers: Record; - body: Record; - }; + upstream_response?: RunnerUpstreamResponse; } + +export interface RunnerUpstreamResponse { + status: number; + headers: Record; + body: unknown; +} + export interface RunnerOutput { success: boolean; error: RunnerOutputError | null; From a23ff5638cba936d609e5cb4bfa88e8b14763864 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Wed, 8 Jan 2025 16:59:51 -0500 Subject: [PATCH 06/19] Fix some type errors --- packages/runner/lib/exec.ts | 11 ++++++++--- packages/types/lib/runner/index.ts | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index e11fc7f4b67..8a71fb650df 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -1,5 +1,5 @@ import type { NangoProps } from '@nangohq/shared'; -import { AxiosError } from 'axios'; +import { isAxiosError } from 'axios'; import { ActionError, NangoSync, NangoAction, instrumentSDK, SpanTypes, validateData, NangoError } from '@nangohq/shared'; import { Buffer } from 'buffer'; import * as vm from 'node:vm'; @@ -13,6 +13,11 @@ import { errorToObject, metrics, truncateJson } from '@nangohq/utils'; import { logger } from './utils.js'; import type { RunnerOutput } from '@nangohq/types'; +interface ScriptExports { + onWebhookPayloadReceived?: (nango: NangoAction, payload?: object) => Promise; + default: (nango: NangoAction, payload?: object) => Promise; +} + export async function exec( nangoProps: NangoProps, code: string, @@ -76,7 +81,7 @@ export async function exec( }; const context = vm.createContext(sandbox); - const scriptExports = script.runInContext(context); + const scriptExports = script.runInContext(context) as ScriptExports; if (nangoProps.scriptType === 'webhook') { if (!scriptExports.onWebhookPayloadReceived) { @@ -174,7 +179,7 @@ export async function exec( }, response: null }; - } else if (err instanceof AxiosError) { + } else if (isAxiosError(err)) { span.setTag('error', err); if (err.response?.data) { const errorResponse = err.response.data.payload || err.response.data; diff --git a/packages/types/lib/runner/index.ts b/packages/types/lib/runner/index.ts index be79dc0e79f..00dc42b95a4 100644 --- a/packages/types/lib/runner/index.ts +++ b/packages/types/lib/runner/index.ts @@ -1,6 +1,6 @@ export interface RunnerOutputError { type: string; - payload: Record; + payload: Record | unknown[]; status: number; upstream_response?: RunnerUpstreamResponse; } @@ -14,7 +14,7 @@ export interface RunnerUpstreamResponse { export interface RunnerOutput { success: boolean; error: RunnerOutputError | null; - response?: any; // TODO: define response type + response?: unknown; // TODO: define response type } export interface RunnerFlags { From 48ae65e63d0982000f5637b429c9c03503b89dbe Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Thu, 9 Jan 2025 11:40:47 -0500 Subject: [PATCH 07/19] Add comment about why I'm using isAxiosError --- packages/runner/lib/exec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index 8a71fb650df..e073383bb39 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -180,6 +180,9 @@ export async function exec( response: null }; } else if (isAxiosError(err)) { + // isAxiosError lets us use something the shape of an axios error in + // testing, which is handy with how strongly typed everything is + span.setTag('error', err); if (err.response?.data) { const errorResponse = err.response.data.payload || err.response.data; From 7a397e7df6015ce793fc59c922e4c90a9e2f3a58 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Thu, 9 Jan 2025 12:32:57 -0500 Subject: [PATCH 08/19] Fix all eslint warnings in exec.ts --- packages/runner/lib/exec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index e073383bb39..606278218f9 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -179,17 +179,18 @@ export async function exec( }, response: null }; - } else if (isAxiosError(err)) { + } else if (isAxiosError(err)) { // isAxiosError lets us use something the shape of an axios error in // testing, which is handy with how strongly typed everything is span.setTag('error', err); if (err.response?.data) { - const errorResponse = err.response.data.payload || err.response.data; + const data = err.response.data as Record; + const errorResponse = (data['payload'] || data) as Record; const headers = Object.fromEntries( Object.entries(err.response.headers) - .map(([k, v]) => [k.toLowerCase(), v.toString()]) + .map<[string, string]>(([k, v]) => [k.toLowerCase(), String(v)]) .filter(([k]) => k === 'content-type' || k.startsWith('x-rate')) ); From c43fe833fa2094dc0e9f98fe9cf2d7a228d363a9 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Thu, 9 Jan 2025 14:35:56 -0500 Subject: [PATCH 09/19] Update jobs putTask to handle new error format in validation --- packages/jobs/lib/routes/tasks/putTask.ts | 19 ++++++++++--------- packages/types/lib/runner/index.ts | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/jobs/lib/routes/tasks/putTask.ts b/packages/jobs/lib/routes/tasks/putTask.ts index ba12e21c2aa..1e2d8b36bd4 100644 --- a/packages/jobs/lib/routes/tasks/putTask.ts +++ b/packages/jobs/lib/routes/tasks/putTask.ts @@ -1,5 +1,5 @@ import { z } from 'zod'; -import type { ApiError, Endpoint } from '@nangohq/types'; +import type { ApiError, Endpoint, RunnerOutputError } from '@nangohq/types'; import { validateRequest } from '@nangohq/utils'; import type { EndpointRequest, EndpointResponse, RouteHandler } from '@nangohq/utils'; import { handleError, handleSuccess } from '../../execution/operations/output.js'; @@ -17,13 +17,7 @@ type PutTask = Endpoint<{ }; Body: { nangoProps?: NangoProps; - error?: - | { - type: string; - payload: Record; - status: number; - } - | undefined; + error?: RunnerOutputError | undefined; output: JsonValue; }; Error: ApiError<'put_task_failed'>; @@ -93,7 +87,14 @@ const validate = validateRequest({ .object({ type: z.string(), payload: z.record(z.string(), z.unknown()).or(z.unknown().transform((v) => ({ message: v }))), - status: z.number() + status: z.number(), + upstream_response: z + .object({ + status: z.number(), + headers: z.record(z.string(), z.string()), + body: z.unknown() + }) + .optional() }) .optional(), output: jsonSchema.default(null) diff --git a/packages/types/lib/runner/index.ts b/packages/types/lib/runner/index.ts index 00dc42b95a4..546dd172bab 100644 --- a/packages/types/lib/runner/index.ts +++ b/packages/types/lib/runner/index.ts @@ -2,13 +2,13 @@ export interface RunnerOutputError { type: string; payload: Record | unknown[]; status: number; - upstream_response?: RunnerUpstreamResponse; + upstream_response?: RunnerUpstreamResponse | undefined; } export interface RunnerUpstreamResponse { status: number; headers: Record; - body: unknown; + body?: unknown; } export interface RunnerOutput { From 4cf4721efdac235dacb1e45e273eb64aa64325d9 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Thu, 9 Jan 2025 16:33:29 -0500 Subject: [PATCH 10/19] WIP on figuring out how to get error to flow through orchestrator --- packages/orchestrator/lib/clients/types.ts | 2 ++ packages/shared/lib/clients/orchestrator.ts | 25 +++++++++++---------- packages/shared/lib/utils/error.ts | 5 ++++- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/orchestrator/lib/clients/types.ts b/packages/orchestrator/lib/clients/types.ts index 2f49bab4f38..cfddc91df40 100644 --- a/packages/orchestrator/lib/clients/types.ts +++ b/packages/orchestrator/lib/clients/types.ts @@ -4,6 +4,7 @@ import type { PostRecurring } from '../routes/v1/postRecurring.js'; import type { Result } from '@nangohq/utils'; import type { ScheduleState, TaskState } from '@nangohq/scheduler'; import type { PostScheduleRun } from '../routes/v1/schedules/postRun.js'; +import type { RunnerUpstreamResponse } from '@nangohq/types'; export type ImmediateProps = PostImmediate['Body']; export type RecurringProps = PostRecurring['Body']; @@ -203,4 +204,5 @@ export function TaskOnEvent(props: TaskCommonFields & OnEventArgs): TaskOnEvent export interface ClientError extends Error { name: string; payload: JsonValue; + upstream_response?: RunnerUpstreamResponse; } diff --git a/packages/shared/lib/clients/orchestrator.ts b/packages/shared/lib/clients/orchestrator.ts index ad15935bc2c..0330867b5ec 100644 --- a/packages/shared/lib/clients/orchestrator.ts +++ b/packages/shared/lib/clients/orchestrator.ts @@ -32,6 +32,7 @@ import { getSyncConfigRaw, getSyncConfigBySyncId } from '../services/sync/config import environmentService from '../services/environment.service.js'; import type { DBEnvironment, DBTeam } from '@nangohq/types'; import type { RecordCount } from '@nangohq/records'; +import type { JsonValue } from 'type-fest'; export interface RecordsServiceInterface { deleteRecordsBySyncId({ @@ -98,7 +99,7 @@ export class Orchestrator { return Ok(scheduleMap); } - async triggerAction({ + async triggerAction({ connection, actionName, input, @@ -117,13 +118,16 @@ export class Orchestrator { 'connection.provider_config_key': connection.provider_config_key, 'connection.environment_id': connection.environment_id }; + if (!connection.id) { + throw new NangoError('invalid_input', { connection }); + } const span = tracer.startSpan('execute.action', { tags: spanTags, ...(activeSpan ? { childOf: activeSpan } : {}) }); const startTime = Date.now(); try { - let parsedInput = null; + let parsedInput: JsonValue = null; try { parsedInput = input ? JSON.parse(JSON.stringify(input)) : null; } catch (err) { @@ -136,7 +140,7 @@ export class Orchestrator { const args = { actionName, connection: { - id: connection.id!, + id: connection.id, connection_id: connection.connection_id, provider_config_key: connection.provider_config_key, environment_id: connection.environment_id @@ -153,7 +157,10 @@ export class Orchestrator { const res = actionResult.mapError((err) => { return ( deserializeNangoError(err.payload) || - new NangoError('action_script_failure', { error: err.message, ...(err.payload ? { payload: err.payload } : {}) }) + new NangoError('action_script_failure', { + error: err.message, + ...(err.payload ? { payload: err.payload } : {}) + }) ); }); @@ -241,7 +248,7 @@ export class Orchestrator { } } - async triggerWebhook({ + async triggerWebhook({ account, environment, integration, @@ -323,12 +330,6 @@ export class Orchestrator { throw res.error; } - await logCtx.info('The webhook was successfully run', { - action: webhookName, - connection: connection.connection_id, - integration: connection.provider_config_key - }); - await logCtx.success(); metrics.increment(metrics.Types.WEBHOOK_SUCCESS); @@ -370,7 +371,7 @@ export class Orchestrator { } } - async triggerOnEventScript({ + async triggerOnEventScript({ connection, version, name, diff --git a/packages/shared/lib/utils/error.ts b/packages/shared/lib/utils/error.ts index 2f36f29a536..d32220264e2 100644 --- a/packages/shared/lib/utils/error.ts +++ b/packages/shared/lib/utils/error.ts @@ -1,16 +1,19 @@ import { stringifyError } from '@nangohq/utils'; +import type { JsonValue } from 'type-fest'; export class NangoError extends Error { public readonly status: number = 500; public readonly type: string; public payload: Record; + public additionalProperties: JsonValue = {}; public override readonly message: string; - constructor(type: string, payload = {}, status?: number) { + constructor(type: string, payload = {}, status?: number, additionalProperties: JsonValue = {}) { super(); this.type = type; this.payload = payload; + this.additionalProperties = additionalProperties; if (status) { this.status = status; From 2ab655a4ead3571bcb9545be6656e73945bef2fc Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Fri, 10 Jan 2025 15:07:26 -0500 Subject: [PATCH 11/19] Rework error to have additional_properties --- packages/runner/lib/exec.ts | 10 ++++++---- packages/runner/lib/exec.unit.test.ts | 20 +++++++++++--------- packages/shared/lib/utils/error.ts | 8 ++++---- packages/types/lib/runner/index.ts | 6 +++++- vite.config.ts | 3 ++- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index 606278218f9..d5b0b1e6bfe 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -200,10 +200,12 @@ export async function exec( type: 'script_http_error', payload: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse), status: err.response.status, - upstream_response: { - status: err.response.status, - headers, - body: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse) + additional_properties: { + upstream_response: { + status: err.response.status, + headers, + body: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse) + } } }, response: null diff --git a/packages/runner/lib/exec.unit.test.ts b/packages/runner/lib/exec.unit.test.ts index da05244bd1a..3a300738e58 100644 --- a/packages/runner/lib/exec.unit.test.ts +++ b/packages/runner/lib/exec.unit.test.ts @@ -186,15 +186,17 @@ describe('Exec', () => { error: 'Not found' }, status: 404, - upstream_response: { - body: { - error: 'Not found' - }, - headers: { - 'content-type': 'application/json', - 'x-ratelimit-limit': '100' - }, - status: 404 + additional_properties: { + upstream_response: { + body: { + error: 'Not found' + }, + headers: { + 'content-type': 'application/json', + 'x-ratelimit-limit': '100' + }, + status: 404 + } }, type: 'script_http_error' }); diff --git a/packages/shared/lib/utils/error.ts b/packages/shared/lib/utils/error.ts index d32220264e2..54aff0d5d4e 100644 --- a/packages/shared/lib/utils/error.ts +++ b/packages/shared/lib/utils/error.ts @@ -5,15 +5,15 @@ export class NangoError extends Error { public readonly status: number = 500; public readonly type: string; public payload: Record; - public additionalProperties: JsonValue = {}; + public additional_properties: JsonValue = {}; public override readonly message: string; - constructor(type: string, payload = {}, status?: number, additionalProperties: JsonValue = {}) { + constructor(type: string, payload = {}, status?: number, additional_properties: JsonValue = {}) { super(); this.type = type; this.payload = payload; - this.additionalProperties = additionalProperties; + this.additional_properties = additional_properties; if (status) { this.status = status; @@ -525,7 +525,7 @@ export function isNangoErrorAsJson(obj: unknown): obj is NangoError { export function deserializeNangoError(err: unknown): NangoError | null { if (isNangoErrorAsJson(err)) { - return new NangoError(err['type'], err.payload, err.status); + return new NangoError(err['type'], err.payload, err.status, err.additional_properties); } return null; } diff --git a/packages/types/lib/runner/index.ts b/packages/types/lib/runner/index.ts index 546dd172bab..a171b4898b4 100644 --- a/packages/types/lib/runner/index.ts +++ b/packages/types/lib/runner/index.ts @@ -2,7 +2,11 @@ export interface RunnerOutputError { type: string; payload: Record | unknown[]; status: number; - upstream_response?: RunnerUpstreamResponse | undefined; + additional_properties?: + | { + upstream_response?: RunnerUpstreamResponse | undefined; + } + | undefined; } export interface RunnerUpstreamResponse { diff --git a/vite.config.ts b/vite.config.ts index eac1fd44891..7e26ce7e7d2 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -13,7 +13,8 @@ export default defineConfig({ NANGO_LOGS_ES_URL: 'http://fake.com', NANGO_LOGS_ES_USER: '', NANGO_LOGS_ES_PWD: '', - NANGO_LOGS_ENABLED: 'false' + NANGO_LOGS_ENABLED: 'false', + NANGO_DB_NAME: 'nango_test' }, chaiConfig: { truncateThreshold: 10000 From d06af85dd4527deb3ca02f7d33cbd597cc31e618 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Fri, 10 Jan 2025 15:46:03 -0500 Subject: [PATCH 12/19] Got upstream errors flowing out of action errors --- packages/jobs/lib/routes/tasks/putTask.ts | 8 +------- packages/orchestrator/lib/clients/client.ts | 4 +++- packages/orchestrator/lib/clients/types.ts | 3 +-- packages/shared/lib/utils/error.manager.ts | 8 ++++++-- packages/shared/lib/utils/error.ts | 11 ++++++++--- packages/types/lib/runner/index.ts | 12 +----------- 6 files changed, 20 insertions(+), 26 deletions(-) diff --git a/packages/jobs/lib/routes/tasks/putTask.ts b/packages/jobs/lib/routes/tasks/putTask.ts index 1e2d8b36bd4..743764beaf8 100644 --- a/packages/jobs/lib/routes/tasks/putTask.ts +++ b/packages/jobs/lib/routes/tasks/putTask.ts @@ -88,13 +88,7 @@ const validate = validateRequest({ type: z.string(), payload: z.record(z.string(), z.unknown()).or(z.unknown().transform((v) => ({ message: v }))), status: z.number(), - upstream_response: z - .object({ - status: z.number(), - headers: z.record(z.string(), z.string()), - body: z.unknown() - }) - .optional() + additional_properties: z.record(z.string(), z.unknown()).optional() }) .optional(), output: jsonSchema.default(null) diff --git a/packages/orchestrator/lib/clients/client.ts b/packages/orchestrator/lib/clients/client.ts index 544ff6e030c..5459bacc1ad 100644 --- a/packages/orchestrator/lib/clients/client.ts +++ b/packages/orchestrator/lib/clients/client.ts @@ -181,6 +181,7 @@ export class OrchestratorClient { retryIf: (res) => 'error' in res && Date.now() < retryUntil } })({ params: { taskId }, query: { longPolling: 30_000 } }); + if ('error' in getOutput) { return Err({ name: getOutput.error.code, @@ -408,7 +409,8 @@ export class OrchestratorClient { name: error.name, type: 'type' in error ? (error.type as string) : 'unknown_error', message: error.message, - payload: 'payload' in error ? (error.payload as any) : null + payload: 'payload' in error ? (error.payload as any) : null, + additional_properties: 'additional_properties' in error ? (error.additional_properties as any) : null }; const res = await this.routeFetch(putTaskRoute)({ params: { taskId }, diff --git a/packages/orchestrator/lib/clients/types.ts b/packages/orchestrator/lib/clients/types.ts index cfddc91df40..0478d576d50 100644 --- a/packages/orchestrator/lib/clients/types.ts +++ b/packages/orchestrator/lib/clients/types.ts @@ -4,7 +4,6 @@ import type { PostRecurring } from '../routes/v1/postRecurring.js'; import type { Result } from '@nangohq/utils'; import type { ScheduleState, TaskState } from '@nangohq/scheduler'; import type { PostScheduleRun } from '../routes/v1/schedules/postRun.js'; -import type { RunnerUpstreamResponse } from '@nangohq/types'; export type ImmediateProps = PostImmediate['Body']; export type RecurringProps = PostRecurring['Body']; @@ -204,5 +203,5 @@ export function TaskOnEvent(props: TaskCommonFields & OnEventArgs): TaskOnEvent export interface ClientError extends Error { name: string; payload: JsonValue; - upstream_response?: RunnerUpstreamResponse; + additional_properties?: Record; } diff --git a/packages/shared/lib/utils/error.manager.ts b/packages/shared/lib/utils/error.manager.ts index 2cc9902ac71..84e8a8fd56f 100644 --- a/packages/shared/lib/utils/error.manager.ts +++ b/packages/shared/lib/utils/error.manager.ts @@ -44,9 +44,13 @@ class ErrorManager { if (err) { logger.error(`Response error`, errorToObject(err)); if (!err.message) { - res.status(err.status || 500).send({ error: { code: err.type || 'unknown_error', payload: err.payload } }); + res.status(err.status || 500).send({ + error: { code: err.type || 'unknown_error', payload: err.payload, additional_properties: err.additional_properties } + }); } else { - res.status(err.status || 500).send({ error: { message: err.message, code: err.type, payload: err.payload } }); + res.status(err.status || 500).send({ + error: { message: err.message, code: err.type, payload: err.payload, additional_properties: err.additional_properties } + }); } } else { res.status(500).json({ error: { code: 'unknown_empty_error' } }); diff --git a/packages/shared/lib/utils/error.ts b/packages/shared/lib/utils/error.ts index 54aff0d5d4e..4b0ba42711c 100644 --- a/packages/shared/lib/utils/error.ts +++ b/packages/shared/lib/utils/error.ts @@ -5,10 +5,10 @@ export class NangoError extends Error { public readonly status: number = 500; public readonly type: string; public payload: Record; - public additional_properties: JsonValue = {}; + public additional_properties: Record = {}; public override readonly message: string; - constructor(type: string, payload = {}, status?: number, additional_properties: JsonValue = {}) { + constructor(type: string, payload = {}, status?: number, additional_properties: Record = {}) { super(); this.type = type; @@ -487,8 +487,13 @@ export class NangoError extends Error { this.message = `An invalid error of type: ${typeof this.payload}`; break; - case 'script_http_error': + case 'script_network_error': this.status = 500; + this.message = 'A network error occurred during an HTTP call'; + break; + + case 'script_http_error': + this.status = 424; this.message = `An error occurred during an HTTP call`; break; diff --git a/packages/types/lib/runner/index.ts b/packages/types/lib/runner/index.ts index a171b4898b4..006722059e6 100644 --- a/packages/types/lib/runner/index.ts +++ b/packages/types/lib/runner/index.ts @@ -2,17 +2,7 @@ export interface RunnerOutputError { type: string; payload: Record | unknown[]; status: number; - additional_properties?: - | { - upstream_response?: RunnerUpstreamResponse | undefined; - } - | undefined; -} - -export interface RunnerUpstreamResponse { - status: number; - headers: Record; - body?: unknown; + additional_properties?: Record | undefined; } export interface RunnerOutput { From d9955080c1005b9e5482f797e3d8af2d5c6425ff Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Fri, 10 Jan 2025 16:12:23 -0500 Subject: [PATCH 13/19] Have additional_properties set to undefined by default --- packages/shared/lib/utils/error.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared/lib/utils/error.ts b/packages/shared/lib/utils/error.ts index 4b0ba42711c..9eea2d8f570 100644 --- a/packages/shared/lib/utils/error.ts +++ b/packages/shared/lib/utils/error.ts @@ -5,10 +5,10 @@ export class NangoError extends Error { public readonly status: number = 500; public readonly type: string; public payload: Record; - public additional_properties: Record = {}; + public additional_properties?: Record | undefined = undefined; public override readonly message: string; - constructor(type: string, payload = {}, status?: number, additional_properties: Record = {}) { + constructor(type: string, payload = {}, status?: number, additional_properties?: Record) { super(); this.type = type; From 4db0b0930723a498be4f707fa3bf92951e5a1c39 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Fri, 10 Jan 2025 16:58:47 -0500 Subject: [PATCH 14/19] Add 424 to openapi spec --- docs-v2/spec.yaml | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/docs-v2/spec.yaml b/docs-v2/spec.yaml index be6c36ebb75..091b7db4e55 100644 --- a/docs-v2/spec.yaml +++ b/docs-v2/spec.yaml @@ -1434,6 +1434,48 @@ paths: message: type: string + '424': + description: Action http error + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + message: + type: string + code: + type: string + payload: + type: object + additional_properties: + type: object + properties: + upstream_response: + type: object + properties: + status_code: + type: integer + headers: + type: object + properties: + content-type: + type: string + patternProperties: + '^x-rate': + type: string + body: + type: object + additionalProperties: true + required: + - message + - code + - payload + required: + - error + /environment-variables: get: description: Retrieve the environment variables as added in the Nango dashboard From 5ecaa067be4942bf13d8a77061dfdb688f36bddb Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Mon, 13 Jan 2025 07:46:04 -0500 Subject: [PATCH 15/19] Re-add mystery code removal. I'm assuming it was eslint formatting while I was fixing types --- packages/shared/lib/clients/orchestrator.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/shared/lib/clients/orchestrator.ts b/packages/shared/lib/clients/orchestrator.ts index 0330867b5ec..c7343edf6a2 100644 --- a/packages/shared/lib/clients/orchestrator.ts +++ b/packages/shared/lib/clients/orchestrator.ts @@ -330,6 +330,12 @@ export class Orchestrator { throw res.error; } + await logCtx.info('The webhook was successfully run', { + action: webhookName, + connection: connection.connection_id, + integration: connection.provider_config_key + }); + await logCtx.success(); metrics.increment(metrics.Types.WEBHOOK_SUCCESS); From b61728bf82e27631eb9ec73fd9b6e771ec9f38dd Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Mon, 13 Jan 2025 07:51:27 -0500 Subject: [PATCH 16/19] Remove note about not reaching instance --- packages/runner/lib/exec.unit.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/runner/lib/exec.unit.test.ts b/packages/runner/lib/exec.unit.test.ts index 3a300738e58..841d2090670 100644 --- a/packages/runner/lib/exec.unit.test.ts +++ b/packages/runner/lib/exec.unit.test.ts @@ -145,8 +145,6 @@ describe('Exec', () => { `; 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' From a39804e0016cf5c3b03f9e22fff73278d38cc397 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Mon, 13 Jan 2025 10:02:59 -0500 Subject: [PATCH 17/19] Move connectionid assertion --- packages/shared/lib/clients/orchestrator.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/shared/lib/clients/orchestrator.ts b/packages/shared/lib/clients/orchestrator.ts index c7343edf6a2..2308e3fe8e7 100644 --- a/packages/shared/lib/clients/orchestrator.ts +++ b/packages/shared/lib/clients/orchestrator.ts @@ -118,15 +118,17 @@ export class Orchestrator { 'connection.provider_config_key': connection.provider_config_key, 'connection.environment_id': connection.environment_id }; - if (!connection.id) { - throw new NangoError('invalid_input', { connection }); - } + const span = tracer.startSpan('execute.action', { tags: spanTags, ...(activeSpan ? { childOf: activeSpan } : {}) }); const startTime = Date.now(); try { + if (!connection.id) { + throw new NangoError('invalid_input', { connection }); + } + let parsedInput: JsonValue = null; try { parsedInput = input ? JSON.parse(JSON.stringify(input)) : null; From 6b665e0c72b40c3c207f96ceceb62c971301c2d4 Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Mon, 13 Jan 2025 10:36:22 -0500 Subject: [PATCH 18/19] Update error response to just have an upstream type --- docs-v2/spec.yaml | 27 +++++++++---------- .../server/lib/controllers/sync.controller.ts | 15 ++++++++++- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/docs-v2/spec.yaml b/docs-v2/spec.yaml index 091b7db4e55..fe9761b34da 100644 --- a/docs-v2/spec.yaml +++ b/docs-v2/spec.yaml @@ -1450,25 +1450,22 @@ paths: type: string payload: type: object - additional_properties: + upstream: type: object properties: - upstream_response: + status_code: + type: integer + headers: type: object properties: - status_code: - type: integer - headers: - type: object - properties: - content-type: - type: string - patternProperties: - '^x-rate': - type: string - body: - type: object - additionalProperties: true + content-type: + type: string + patternProperties: + '^x-rate': + type: string + body: + type: object + additionalProperties: true required: - message - code diff --git a/packages/server/lib/controllers/sync.controller.ts b/packages/server/lib/controllers/sync.controller.ts index 825ee0d185e..fd996fc5aba 100644 --- a/packages/server/lib/controllers/sync.controller.ts +++ b/packages/server/lib/controllers/sync.controller.ts @@ -337,7 +337,20 @@ class SyncController { span.setTag('nango.error', actionResponse.error); await logCtx.failed(); - errorManager.errResFromNangoErr(res, actionResponse.error); + if (actionResponse.error.type === 'script_http_error') { + res.status(424).json({ + error: { + payload: actionResponse.error.payload, + code: actionResponse.error.type, + ...(actionResponse.error.additional_properties && 'upstream_response' in actionResponse.error.additional_properties + ? { upstream: actionResponse.error.additional_properties['upstream_response'] } + : {}) + } + }); + } else { + errorManager.errResFromNangoErr(res, actionResponse.error); + } + span.finish(); return; } From 3e2cb1b6e742bc481dfd4bd6d83367dadd03491c Mon Sep 17 00:00:00 2001 From: Alan Johnson Date: Mon, 13 Jan 2025 11:13:43 -0500 Subject: [PATCH 19/19] Clean up response handling --- packages/runner/lib/exec.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/runner/lib/exec.ts b/packages/runner/lib/exec.ts index d5b0b1e6bfe..5c737e96665 100644 --- a/packages/runner/lib/exec.ts +++ b/packages/runner/lib/exec.ts @@ -184,9 +184,15 @@ export async function exec( // testing, which is handy with how strongly typed everything is span.setTag('error', err); - if (err.response?.data) { - const data = err.response.data as Record; - const errorResponse = (data['payload'] || data) as Record; + if (err.response) { + const maybeData = err.response.data; + + let errorResponse: unknown = {}; + if (maybeData && typeof maybeData === 'object' && 'payload' in maybeData) { + errorResponse = maybeData.payload as Record; + } else { + errorResponse = maybeData; + } const headers = Object.fromEntries( Object.entries(err.response.headers) @@ -194,17 +200,21 @@ export async function exec( .filter(([k]) => k === 'content-type' || k.startsWith('x-rate')) ); + const responseBody: Record = truncateJson( + errorResponse && typeof errorResponse === 'object' ? (errorResponse as Record) : { message: errorResponse } + ); + return { success: false, error: { type: 'script_http_error', - payload: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse), + payload: responseBody, status: err.response.status, additional_properties: { upstream_response: { status: err.response.status, headers, - body: truncateJson(typeof errorResponse === 'string' ? { message: errorResponse } : errorResponse) + body: responseBody } } },