From b8fb81882630aac32251a53fc3c3b56ba53dad99 Mon Sep 17 00:00:00 2001 From: Thomas Bonnin <233326+TBonnin@users.noreply.github.com> Date: Thu, 1 Feb 2024 08:22:46 +0100 Subject: [PATCH] Persist API: more logging and fix tracing error (#1590) * add logging when request to persist API fails * dd-tracer setTag('error', value): value should be an error (not a string) * persist: batchSave/Delete/Update: throw error instead of returning false batch operations results is rarely checked by integration. Throwing an error instead of returning false in order to not fail silently --- .../lib/controllers/persist.controller.ts | 18 +++--- packages/persist/lib/utils/result.ts | 6 +- packages/shared/lib/sdk/sync.ts | 58 +++++++++++++++++-- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/packages/persist/lib/controllers/persist.controller.ts b/packages/persist/lib/controllers/persist.controller.ts index 6ee5d57e8e0..df4a4e8d0e4 100644 --- a/packages/persist/lib/controllers/persist.controller.ts +++ b/packages/persist/lib/controllers/persist.controller.ts @@ -270,16 +270,16 @@ class PersistController { content: `There was an issue with the batch ${persistType}. ${error?.message}`, timestamp: Date.now() }); - const errMsg = `Failed to ${persistType} records ${activityLogId}`; - span.setTag('error', errMsg).finish(); - return err(errMsg); + const res = err(`Failed to ${persistType} records ${activityLogId}`); + span.setTag('error', res.error).finish(); + return res; } const syncConfig = await getSyncConfigByJobId(syncJobId); if (syncConfig && !syncConfig?.models.includes(model)) { - const errMsg = `The model '${model}' is not included in the declared sync models: ${syncConfig.models}.`; - span.setTag('error', errMsg).finish(); - return err(errMsg); + const res = err(`The model '${model}' is not included in the declared sync models: ${syncConfig.models}.`); + span.setTag('error', res.error).finish(); + return res; } const persistResult = await persistFunction(formattedRecords); @@ -333,9 +333,9 @@ class PersistController { syncJobId: syncJobId } }); - const errMsg = persistResult?.error!; - span.setTag('error', errMsg).finish(); - return err(errMsg); + const res = err(persistResult?.error!); + span.setTag('error', res.error).finish(); + return res; } } } diff --git a/packages/persist/lib/utils/result.ts b/packages/persist/lib/utils/result.ts index 4a06d8a2215..99a34e81dd3 100644 --- a/packages/persist/lib/utils/result.ts +++ b/packages/persist/lib/utils/result.ts @@ -1,8 +1,10 @@ export type Result = { ok: true; value: T } | { ok: false; error: E }; -export function ok(value: T): Result { +export type ResultValue = { ok: true; value: T }; +export type ResultError = { ok: false; error: E }; +export function ok(value: T): ResultValue { return { ok: true, value }; } -export function err(errMsg: string): Result { +export function err(errMsg: string): ResultError { const e = new Error(errMsg) as E; return { ok: false, error: e }; } diff --git a/packages/shared/lib/sdk/sync.ts b/packages/shared/lib/sdk/sync.ts index aef64faf343..c55909f99be 100644 --- a/packages/shared/lib/sdk/sync.ts +++ b/packages/shared/lib/sdk/sync.ts @@ -429,7 +429,16 @@ export class NangoAction { } }); if (response.status > 299) { - throw new Error(`cannot write log with activityLogId '${this.activityLogId}'`); + console.log( + `Request to persist API (log) failed: errorCode=${response.status} response='${JSON.stringify(response.data)}'`, + JSON.stringify(this, (key, value) => { + if (key === 'secretKey') { + return '********'; + } + return value; + }) + ); + throw new Error(`cannot save log for activityLogId '${this.activityLogId}'`); } return; } @@ -570,7 +579,19 @@ export class NangoSync extends NangoAction { lastSyncDate: date } }); - return response.status <= 299; + if (response.status > 299) { + console.log( + `Request to persist API (setLastSyncDate) failed: errorCode=${response.status} response='${JSON.stringify(response.data)}'`, + JSON.stringify(this, (key, value) => { + if (key === 'secretKey') { + return '********'; + } + return value; + }) + ); + throw new Error(`cannot set lastSyncDate for sync '${this.syncId}'`); + } + return true; } else { return await setLastSyncDate(this.syncId as string, date); } @@ -619,7 +640,16 @@ export class NangoSync extends NangoAction { } }); if (response.status > 299) { - return false; + console.log( + `Request to persist API (batchSave) failed: errorCode=${response.status} response='${JSON.stringify(response.data)}'`, + JSON.stringify(this, (key, value) => { + if (key === 'secretKey') { + return '********'; + } + return value; + }) + ); + throw new Error(`cannot save records for sync '${this.syncId}'`); } } return true; @@ -756,7 +786,16 @@ export class NangoSync extends NangoAction { } }); if (response.status > 299) { - return false; + console.log( + `Request to persist API (batchDelete) failed: errorCode=${response.status} response='${JSON.stringify(response.data)}'`, + JSON.stringify(this, (key, value) => { + if (key === 'secretKey') { + return '********'; + } + return value; + }) + ); + throw new Error(`cannot delete records for sync '${this.syncId}'`); } } return true; @@ -895,7 +934,16 @@ export class NangoSync extends NangoAction { } }); if (response.status > 299) { - return false; + console.log( + `Request to persist API (batchUpdate) failed: errorCode=${response.status} response='${JSON.stringify(response.data)}'`, + JSON.stringify(this, (key, value) => { + if (key === 'secretKey') { + return '********'; + } + return value; + }) + ); + throw new Error(`cannot update records for sync '${this.syncId}'`); } } return true;