feat: booking errors logging #12325
Merged
feat: booking errors logging #12325
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
Contributor
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes! |
Contributor
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
Current Playwright Test Results Summary✅ 15 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 11/15/2023 07:50:09pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 4fe5d0b Started: 11/15/2023 07:48:59pm UTC
|
|
|
2 Test Cases Affected |
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Popup Tests should open embed iframe on click - Configured with light theme
Retry 1 • Initial Attempt |
1.22% (4)4 / 329 runsfailed over last 7 days |
56.23% (185)185 / 329 runsflaked over last 7 days |
|
Popup Tests should be able to reschedule
Retry 2 • Retry 1 • Initial Attempt |
17.38% (57)57 / 328 runsfailed over last 7 days |
77.74% (255)255 / 328 runsflaked over last 7 days |
Udit-takkar
commented
Nov 13, 2023
Comment on lines
352
to
365
| if (!updatedResult) { | ||
| logger.error( | ||
| "updateEvent failed", | ||
| safeStringify({ | ||
| success, | ||
| bookingRefUid, | ||
| credential: getPiiFreeCredential(credential), | ||
| originalEvent: getPiiFreeCalendarEvent(calEvent), | ||
| calError, | ||
| }) | ||
| ); | ||
| throw new Error("update_calendar_event_error"); | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
This would throw error if it is unable to update calendar event and you would get the error message on the booking page
Udit-takkar
commented
Nov 15, 2023
| const fullName = getFullName(bookerName); | ||
|
|
||
| // Why are we only using "en" locale | ||
| const tGuests = await getTranslation("en", "common"); |
Contributor
Author
There was a problem hiding this comment.
Why are we only using "en" locale?
zomars
reviewed
Nov 15, 2023
zomars
approved these changes
Nov 15, 2023
Contributor
zomars
left a comment
zomars
left a comment
There was a problem hiding this comment.
Huge improvement @Udit-takkar Nice work 🙏🏽
Contributor
jakazzy
pushed a commit
to jakazzy/cal.com
that referenced
this pull request
Dec 5, 2023
Fixes: calcom#12297 Fixes calcom#11234 - Displaying error message and X-Vercel-Id( Unique Request Id ) to user on book event form - Improve error logging - Add Error codes Few things to discuss 1) How to handle calendar integration failures ? Currently if for example google integration is broken and someone is trying to book that person then we log the error but don't inform the user that the google calendar is broken and the meeting goes through. Should I throw error when integration is broken ? <img width="758" alt="Screenshot 2023-11-12 at 12 52 36 AM" src="https://github.com/calcom/cal.com/assets/53316345/c4d921c4-9c8a-4b9b-82a2-bbe0fdbcb3d4"> 2) How to handle conferencing app failures? We just default to Cal Video as location if we are unable to generated conferencing url and log the error and not inform the user(organizer).
zomars
pushed a commit
that referenced
this pull request
Jan 29, 2024
Fixes: #12297 Fixes #11234 - Displaying error message and X-Vercel-Id( Unique Request Id ) to user on book event form - Improve error logging - Add Error codes Few things to discuss 1) How to handle calendar integration failures ? Currently if for example google integration is broken and someone is trying to book that person then we log the error but don't inform the user that the google calendar is broken and the meeting goes through. Should I throw error when integration is broken ? <img width="758" alt="Screenshot 2023-11-12 at 12 52 36 AM" src="https://github.com/calcom/cal.com/assets/53316345/c4d921c4-9c8a-4b9b-82a2-bbe0fdbcb3d4"> 2) How to handle conferencing app failures? We just default to Cal Video as location if we are unable to generated conferencing url and log the error and not inform the user(organizer).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #12297
Fixes #11234
Few things to discuss
Currently if for example google integration is broken and someone is trying to book that person then we log the error but don't inform the user that the google calendar is broken and the meeting goes through.
Should I throw error when integration is broken ?
We just default to Cal Video as location if we are unable to generated conferencing url and log the error and not inform the user(organizer).