Skip to content

fix: unhandled promise rejection in scheduleWorkflowReminder#12301

Merged
hariombalhara merged 8 commits into
mainfrom
fix/unhandled-promise-rejection
Nov 14, 2023
Merged

fix: unhandled promise rejection in scheduleWorkflowReminder#12301
hariombalhara merged 8 commits into
mainfrom
fix/unhandled-promise-rejection

Conversation

@CarinaWolli
Copy link
Copy Markdown
Member

@CarinaWolli CarinaWolli commented Nov 9, 2023

What does this PR do?

Fixes unhandled promise rejection error that was caused by await before Promise.allSettled.

Now we first load all needed workflowReminders and after that we loop over all workflowReminders and create the promises.

fixes #12340

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2023 10:13pm
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2023 10:13pm
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ai ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 10:13pm
cal ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 10:13pm
cal-demo ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 10:13pm
qa ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 10:13pm
ui ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 10:13pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 9, 2023

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes!

@zomars zomars added the core area: core, team members only label Nov 9, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 9, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Nov 9, 2023

Current Playwright Test Results Summary

✅ 313 Passing - ⚠️ 10 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 11/13/2023 10:23:13pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: ff4f694

Started: 11/13/2023 10:14:50pm UTC

⚠️ Flakes

📄   apps/web/playwright/integrations-stripe.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Stripe integration Can book a paid booking
Retry 1Initial Attempt
0.35% (1) 1 / 285 run
failed over last 7 days
1.40% (4) 4 / 285 runs
flaked over last 7 days

📄   apps/web/playwright/event-types.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Event Types tests user Different Locations Tests Can add Organzer Phone Number location and book with it
Retry 1Initial Attempt
0.68% (2) 2 / 292 runs
failed over last 7 days
4.45% (13) 13 / 292 runs
flaked over last 7 days
Event Types tests user Different Locations Tests Can add Link Meeting as location and book with it
Retry 1Initial Attempt
0.69% (2) 2 / 290 runs
failed over last 7 days
6.90% (20) 20 / 290 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe according to system theme when no theme is configured through Embed API
Retry 1Initial Attempt
1.33% (4) 4 / 300 runs
failed over last 7 days
54.33% (163) 163 / 300 runs
flaked over last 7 days
Popup Tests should be able to reschedule
Retry 1Initial Attempt
16.67% (50) 50 / 300 runs
failed over last 7 days
77.67% (233) 233 / 300 runs
flaked over last 7 days

📄   apps/web/playwright/webhook.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
FORM_SUBMITTED on submitting team form, triggers team webhook
Retry 1Initial Attempt
1.04% (3) 3 / 289 runs
failed over last 7 days
12.80% (37) 37 / 289 runs
flaked over last 7 days

📄   apps/web/playwright/managed-event-types.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Managed Event Types tests Can create managed event type
Retry 1Initial Attempt
9.69% (28) 28 / 289 runs
failed over last 7 days
20.07% (58) 58 / 289 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/inline.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Inline Iframe Inline Iframe - Configured with Dark Theme
Retry 1Initial Attempt
0.68% (2) 2 / 293 runs
failed over last 7 days
38.23% (112) 112 / 293 runs
flaked over last 7 days

📄   packages/app-store/routing-forms/playwright/tests/basic.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Routing Forms Seeded Routing Form Router URL should work
Retry 1Initial Attempt
0.35% (1) 1 / 288 run
failed over last 7 days
13.89% (40) 40 / 288 runs
flaked over last 7 days

📄   apps/web/playwright/reschedule.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Reschedule Tests Should display request reschedule send on bookings/cancelled
Retry 1Initial Attempt
0.67% (2) 2 / 298 runs
failed over last 7 days
0.34% (1) 1 / 298 run
flaked over last 7 days

View Detailed Build Results


Comment thread packages/features/ee/workflows/api/scheduleEmailReminders.ts Outdated
Comment thread packages/features/ee/workflows/api/scheduleEmailReminders.ts Outdated
Comment thread packages/features/ee/workflows/api/scheduleEmailReminders.ts Outdated
Comment thread packages/features/ee/workflows/api/scheduleEmailReminders.ts Outdated
Comment thread packages/features/ee/workflows/api/scheduleEmailReminders.ts
Comment thread packages/features/ee/workflows/api/scheduleEmailReminders.ts Outdated
Comment thread packages/features/ee/workflows/api/scheduleEmailReminders.ts
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach looks correct to me now @CarinaWolli. Left a few comments to make the code more DRY and comfortable to understand.

Also, I think it could make sense to unit test it as it seems like an important endpoint to me(Could be a followup)

@CarinaWolli
Copy link
Copy Markdown
Member Author

@hariombalhara Thank you for the feedback. I made the adjustments to the code, can you take a look again?

Also, I will follow up with some tests: #12355

Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now !!

@hariombalhara hariombalhara merged commit a2f859b into main Nov 14, 2023
@hariombalhara hariombalhara deleted the fix/unhandled-promise-rejection branch November 14, 2023 04:57
zomars pushed a commit that referenced this pull request Jan 29, 2024
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only High priority Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CAL-2703] Fix Cron job scheduleEmailReminders

3 participants