-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add error handling service for calendar import #6203
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
- Added
CalendarEventImportErrorHandlerService
tocalendar-event-import-manager.module.ts
- Introduced
CALENDAR_THROTTLE_DURATION
andCALENDAR_THROTTLE_MAX_ATTEMPTS
constants - Removed
syncCursor
update logic fromGoogleCalendarGetEventsService
- Added
parseGoogleCalendarError
utility function - Created
calendar-event-import-error-handling.service.ts
for structured error handling
8 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Added
handleError
method ingoogle-calendar-get-events.service.ts
for robust error management - Introduced
GoogleCalendarError
type ingoogle-calendar-error.type.ts
for standardized error objects - Added
parseGaxiosError
utility inparse-gaxios-error.util.ts
for mapping Gaxios errors - Updated
parse-google-calendar-error.util.ts
to useGoogleCalendarError
type for better type safety
No major changes found since last review.
4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Removed
calendarChannelRepository
dependency ingoogle-calendar-get-events.service.ts
- Added error handling logic in
getCalendarEvents
method - Introduced utility function in
parse-google-calendar-error.util.ts
for parsing Google Calendar errors - Categorized errors into types like
INSUFFICIENT_PERMISSIONS
,TEMPORARY_ERROR
,NOT_FOUND
, andUNKNOWN
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Introduced
CalendarEventImportSyncStep
enum for standardized sync steps - Modified error handling logic to use the new enum
- Added
handleNotFoundError
method for specific 'NOT_FOUND' error handling based on sync step
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Enhanced error handling for calendar event import
- Utilized
CalendarEventImportErrorHandlerService
- Differentiated between full and partial fetches using
CalendarEventImportSyncStep
andCalendarChannelSyncStage
enums
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Updated error handling in
packages/twenty-server/src/modules/calendar/calendar-event-import-manager/services/calendar-events-import.service.ts
- Replaced manual error creation with
CalendarEventImportErrorHandlerService
- Simplified error handling process for better maintainability
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
…tialMessageListFetch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Added
markAsCompletedAndSchedulePartialMessageListFetch
method inpackages/twenty-server/src/modules/calendar/calendar-event-import-manager/services/calendar-channel-sync-status.service.ts
- Integrated
CalendarEventImportErrorHandlerService
inpackages/twenty-server/src/modules/calendar/calendar-event-import-manager/services/calendar-events-import.service.ts
- Replaced
schedulePartialCalendarEventListFetch
withmarkAsCompletedAndSchedulePartialMessageListFetch
incalendar-events-import.service.ts
- Reset throttle failure count and sync stage start time in
calendar-channel-sync-status.service.ts
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Added Kubernetes and Terraform configurations for deploying TwentyCRM (
packages/twenty-docker/k8s/
). - Enhanced error handling for link deletion in
LinksFieldInput.tsx
andLinksFieldMenuItem.tsx
. - Updated
absoluteUrlSchema.ts
to allow empty strings as valid input. - Improved error handling in
0-22-update-boolean-fields-null-default-values-and-null-values.command.ts
. - Removed
WorkspaceGate
decorator inconnected-account.workspace-entity.ts
.
32 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Updated cron pattern to run every minute (
packages/twenty-server/src/modules/calendar/calendar-event-import-manager/crons/commands/calendar-event-list-fetch.cron.command.ts
) - Enhanced error handling logic for Google Calendar API responses (
packages/twenty-server/src/modules/calendar/calendar-event-import-manager/drivers/google-calendar/services/google-calendar-get-events.service.ts
)
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Updated cron pattern to run every five minutes (
packages/twenty-server/src/modules/calendar/calendar-event-import-manager/crons/commands/calendar-event-list-fetch.cron.command.ts
)
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is way better! Just added a neat
@@ -90,4 +77,34 @@ export class GoogleCalendarGetEventsService { | |||
nextSyncCursor: nextSyncToken || '', | |||
}; | |||
} | |||
|
|||
private handleError(error: GaxiosError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can improve the naming here as handleError is actually a throwCalendarEventError
) { | ||
throw parseGaxiosError(error); | ||
} | ||
if (error.response?.status !== 410) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this if condition needs some details to be added. Why do we ignore 410? Also, it's preferrable to use a if (edgeCase) { return } pattern instead of if (!edgeCase) which leads to nested if / elses that are harder to read
throw parseGaxiosError(error); | ||
} | ||
if (error.response?.status !== 410) { | ||
const googleCalendarError: GoogleCalendarError = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lastly, why do we do this construction here and not in parseGoogleCalendarError function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Added error handling service for calendar imports (
packages/twenty-server/src/engine/core-modules/graphql/engine-graphql.module.ts
,packages/twenty-server/src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook.ts
) - Updated address fields to structured format across multiple modules (
packages/twenty-server/src/database/typeorm-seeds/workspace/companies.ts
,packages/twenty-server/src/modules/company/standard-objects/company.workspace-entity.ts
) - Removed
Timeline.tsx
component (packages/twenty-front/src/modules/activities/timeline/components/Timeline.tsx
) - Introduced
isCancelDisabled
prop toSaveAndCancelButtons
component (packages/twenty-front/src/pages/settings/data-model/SettingsObjectEdit.tsx
) - Enhanced Recoil and Apollo Client integration for activity creation (
packages/twenty-front/src/modules/activities/hooks/useCreateActivityInDB.ts
)
94 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Add error handling service for calendar import