Skip to content

Commit

Permalink
Persist API: more logging and fix tracing error (#1590)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
TBonnin authored Feb 1, 2024
1 parent e22bd0c commit b8fb818
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 16 deletions.
18 changes: 9 additions & 9 deletions packages/persist/lib/controllers/persist.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions packages/persist/lib/utils/result.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
export type Result<T, E = Error> = { ok: true; value: T } | { ok: false; error: E };
export function ok<T>(value: T): Result<T> {
export type ResultValue<T> = { ok: true; value: T };
export type ResultError<E extends Error> = { ok: false; error: E };
export function ok<T>(value: T): ResultValue<T> {
return { ok: true, value };
}
export function err<E extends Error>(errMsg: string): Result<never, E> {
export function err<E extends Error>(errMsg: string): ResultError<E> {
const e = new Error(errMsg) as E;
return { ok: false, error: e };
}
58 changes: 53 additions & 5 deletions packages/shared/lib/sdk/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit b8fb818

Please sign in to comment.