-
Notifications
You must be signed in to change notification settings - Fork 11.1k
fix: Return error message from getConnectedCalendar
#25338
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
base: main
Are you sure you want to change the base?
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.
No issues found across 1 file
E2E results are ready! |
| }; | ||
| } catch (error) { | ||
| let errorMessage = "Could not get connected calendars"; | ||
| let errorMessage = error; |
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.
are we okay exposing this to the end user?
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
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.
1 issue found across 1 file
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/features/calendars/lib/CalendarManager.ts">
<violation number="1" location="packages/features/calendars/lib/CalendarManager.ts:124">
`errorMessage` is now initialized with the raw `error` object, but it is only converted to a string inside the `invalid_grant` and `CalendarAppDelegationCredentialError` branches. For every other error, `errorMessage` remains an `Error` instance, so the returned `{ error: { message: errorMessage } }` serializes to `{}` because `Error` properties are non-enumerable. Clients therefore lose the actual failure reason, which is the opposite of the PR goal of returning the calendar error message.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| }; | ||
| } catch (error) { | ||
| let errorMessage = "Could not get connected calendars"; | ||
| let errorMessage = error; |
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.
errorMessage is now initialized with the raw error object, but it is only converted to a string inside the invalid_grant and CalendarAppDelegationCredentialError branches. For every other error, errorMessage remains an Error instance, so the returned { error: { message: errorMessage } } serializes to {} because Error properties are non-enumerable. Clients therefore lose the actual failure reason, which is the opposite of the PR goal of returning the calendar error message.
Prompt for AI agents
Address the following comment on packages/features/calendars/lib/CalendarManager.ts at line 124:
<comment>`errorMessage` is now initialized with the raw `error` object, but it is only converted to a string inside the `invalid_grant` and `CalendarAppDelegationCredentialError` branches. For every other error, `errorMessage` remains an `Error` instance, so the returned `{ error: { message: errorMessage } }` serializes to `{}` because `Error` properties are non-enumerable. Clients therefore lose the actual failure reason, which is the opposite of the PR goal of returning the calendar error message.</comment>
<file context>
@@ -122,7 +121,7 @@ export const getConnectedCalendars = async (
};
} catch (error) {
- let errorMessage = "Could not get connected calendars";
+ let errorMessage = error;
// Here you can expect for specific errors
</file context>
| let errorMessage = error; | |
| let errorMessage = error instanceof Error ? error.message : typeof error === "string" ? error : "Could not get connected calendars"; |
What does this PR do?
getConnectedCalendarMandatory Tasks (DO NOT REMOVE)