feat(webhooks): [nan 1064] webhook on sync error #2281
Conversation
| @@ -0,0 +1,153 @@ | |||
| /* eslint-disable @typescript-eslint/unbound-method */ | |||
There was a problem hiding this comment.
Copied over from shared
|
|
||
| dayjs.extend(utc); | ||
|
|
||
| export const sendSync = async ({ |
There was a problem hiding this comment.
Logic essentially copied over from shared
| @@ -103,6 +106,7 @@ export default class SyncRun { | |||
| recordsService: RecordsServiceInterface; | |||
| dryRunService?: NangoProps['dryRunService']; | |||
| slackNotificationService?: SlackService; | |||
| sendSyncWebhook?: (params: SendSyncParams) => Promise<void>; | |||
There was a problem hiding this comment.
I think I am gonna propose to Bastien a refactor run.service project because it has become a mammoth class that does everything. Its run function is 500+ lines I think
There was a problem hiding this comment.
Yeah, it is a beast. Definitely needed.
| @@ -4,12 +4,10 @@ import dayjs from 'dayjs'; | |||
| import utc from 'dayjs/plugin/utc.js'; | |||
There was a problem hiding this comment.
what still live in this file? Is everything moved over to the webhooks package?
There was a problem hiding this comment.
webhook forwarding logic which I'll bring over in another PR.
| export interface ErrorPayload { | ||
| type: string; | ||
| description: string; | ||
| } |
There was a problem hiding this comment.
Is this an api related type?
There was a problem hiding this comment.
internal API I would say
There was a problem hiding this comment.
if we can keep this file for http api, maybe rename the file if clearer 👌🏻
| export interface ErrorPayload { | ||
| type: string; | ||
| description: string; | ||
| } |
There was a problem hiding this comment.
if we can keep this file for http api, maybe rename the file if clearer 👌🏻
| @@ -420,11 +461,10 @@ export default class SyncRun { | |||
| } | |||
|
|
|||
| const startTime = Date.now(); | |||
| const syncStartDate = new Date(); | |||
There was a problem hiding this comment.
Could be moved at the very beginning of the method or could be a property of the class directly
There was a problem hiding this comment.
Will make a note to follow up on this later
Describe your changes
Add in logic to send webhooks on sync failure
Issue ticket number and link
NAN-1064
Checklist before requesting a review (skip if just adding/editing APIs & templates)