Skip to content

fix: unhandled promise error in scheduleEmailReminders endpoint#12183

Merged
CarinaWolli merged 5 commits intomainfrom
fix/stablize-workflow-reminder-endpoint
Nov 6, 2023
Merged

fix: unhandled promise error in scheduleEmailReminders endpoint#12183
CarinaWolli merged 5 commits intomainfrom
fix/stablize-workflow-reminder-endpoint

Conversation

@CarinaWolli
Copy link
Copy Markdown
Member

@CarinaWolli CarinaWolli commented Nov 1, 2023

What does this PR do?

  • Fixes 'Unhandled Promise Rejection'
  • Add more comments to improve readability
  • Move getiCalEventAsString to separate file for better readability of endpoint

Requirement/Documentation

  • If there is a requirement document, please, share it here.
  • If there is ab UI/UX design document, please, share it here.

Type of change

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

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 1, 2023

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

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 1, 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!

};
}>;

function getiCalEventAsString(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

moved this function to /lib folder to improve readability of this enpoint

import { parseRecurringEvent } from "@calcom/lib";
import type { Prisma, User } from "@calcom/prisma/client";

type Booking = Prisma.BookingGetPayload<{
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

just copied everything form above

}

deletePromises.push(
remindersToDelete.map((reminder) =>
Copy link
Copy Markdown
Member Author

@CarinaWolli CarinaWolli Nov 1, 2023

Choose a reason for hiding this comment

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

Resulted in a two-dimensional array and this caused issues with Promise.allSettled not working correctly (Unhandled Promise Rejection)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 1, 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 1, 2023

Current Playwright Test Results Summary

✅ 251 Passing - ⚠️ 3 Flaky

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

(Last updated on 11/03/2023 01:37:29pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 801f14d

Started: 11/03/2023 01:32:53pm UTC

⚠️ Flakes

📄   apps/web/playwright/booking/longTextQuestion.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
Booking With Long Text Question and Each Other Question Booking With Long Text Question and checkbox Question Long Text and checkbox required
Retry 1Initial Attempt
0% (0) 0 / 228 runs
failed over last 7 days
0.88% (2) 2 / 228 runs
flaked over last 7 days
Booking With Long Text Question and Each Other Question Long Text required and Number not required
Retry 1Initial Attempt
0% (0) 0 / 228 runs
failed over last 7 days
0.88% (2) 2 / 228 runs
flaked over last 7 days

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 1Initial Attempt
21.71% (61) 61 / 281 runs
failed over last 7 days
75.44% (212) 212 / 281 runs
flaked over last 7 days

View Detailed Build Results


@CarinaWolli CarinaWolli marked this pull request as ready for review November 1, 2023 16:14
@CarinaWolli CarinaWolli requested a review from a team November 1, 2023 16:15
@CarinaWolli CarinaWolli added 🐛 bug Something isn't working Medium priority Created by Linear-GitHub Sync labels Nov 1, 2023
@CarinaWolli CarinaWolli requested a review from emrysal November 2, 2023 14:23
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

NIT: use logger.error instead of console.log

 Promise.allSettled(allPromisesCancelReminders).then((results) => {
    results.forEach((result) => {
      if (result.status === "rejected") {
        console.log(`Error cancelling scheduled_sends: ${result.reason}`);
      }
    });
  });

code LGTM

@CarinaWolli CarinaWolli merged commit e11cb9e into main Nov 6, 2023
@CarinaWolli CarinaWolli deleted the fix/stablize-workflow-reminder-endpoint branch November 6, 2023 14:14
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

🐛 bug Something isn't working 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.

3 participants