Skip to content

Comments

feat: Pass abort signal to fetch in email validation provider#24289

Merged
hariombalhara merged 2 commits into10-01-feat_zero-bounce-email-validationfrom
devin/pass-abort-signal-1759742148
Oct 6, 2025
Merged

feat: Pass abort signal to fetch in email validation provider#24289
hariombalhara merged 2 commits into10-01-feat_zero-bounce-email-validationfrom
devin/pass-abort-signal-1759742148

Conversation

@hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Oct 6, 2025

What does this PR do?

This PR addresses the GitHub comment from @Udit-takkar on PR #24196 by implementing an abort mechanism for email validation requests. The changes ensure that network requests to the ZeroBounce API can be properly cancelled when the timeout is reached, preventing resource leaks.

Key Changes:

  • Modified IEmailValidationProviderService interface to accept an object parameter {request, abortSignal} instead of separate arguments
  • Updated ZeroBounceEmailValidationProviderService to pass the AbortSignal to the fetch call
  • Updated EmailValidationService to provide the abort signal when calling the provider
  • Renamed parameter from signal to abortSignal for clarity
  • Updated all test cases to match the new object parameter syntax

Link to Devin run: https://app.devin.ai/sessions/c45ece61f03b45a39e358c8be7acf0d4
Requested by: @hariombalhara

Visual Demo (For contributors especially)

N/A - This is an internal API change with no visual components.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - Internal API change with no documentation impact.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Environment Setup:

  • Set ZEROBOUNCE_API_KEY environment variable for integration testing
  • No special test data required

Key Test Scenarios:

  1. Timeout behavior: Verify that when email validation times out (after 3000ms), the fetch request is properly aborted
  2. Normal operation: Ensure email validation still works correctly when requests complete within the timeout
  3. Error handling: Confirm that AbortError is handled appropriately when signals are aborted

Testing Commands:

# Run unit tests
TZ=UTC yarn test packages/features/emailValidation/lib/service/

# Type checking
yarn type-check:ci --force

Checklist

⚠️ Important for Review:

  • Verify this is the only implementation of IEmailValidationProviderService in the codebase
  • Search for any other callers of validateEmail method that might have been missed
  • Test abort functionality - manually verify that network requests are cancelled when timeout occurs
  • Confirm backward compatibility - ensure optional abortSignal parameter works when not provided

Technical Details:

  • Interface breaking change is intentional and necessary for proper abort handling
  • Object parameter pattern {request, abortSignal} is more extensible than separate arguments
  • All existing tests updated to match new signature
  • Parameter renamed from signal to abortSignal for clarity about its purpose with fetch requests

- Update IEmailValidationProviderService interface to accept optional signal parameter
- Pass signal through to fetch call in ZeroBounceEmailValidationProviderService
- Wire signal from AbortController in EmailValidationService to provider

This prevents network requests from continuing after timeout, avoiding resource leaks.

Addresses review comment from Udit-takkar on PR #24196

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Oct 6, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 6, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/pass-abort-signal-1759742148

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- Rename parameter from 'signal' to 'abortSignal' to clearly convey it's for aborting fetch requests
- Update interface, implementation, and call site
- Update tests to match new object parameter syntax

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
@pull-request-size pull-request-size bot added size/M and removed size/XS labels Oct 6, 2025
@vercel
Copy link

vercel bot commented Oct 6, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Oct 6, 2025 11:06am
cal-eu Ignored Ignored Oct 6, 2025 11:06am

@hariombalhara hariombalhara marked this pull request as ready for review October 6, 2025 12:59
@hariombalhara hariombalhara merged commit 45f007d into 10-01-feat_zero-bounce-email-validation Oct 6, 2025
15 checks passed
@hariombalhara hariombalhara deleted the devin/pass-abort-signal-1759742148 branch October 6, 2025 12:59
@graphite-app graphite-app bot requested a review from a team October 6, 2025 12:59
@dosubot dosubot bot added the ✨ feature New feature or request label Oct 6, 2025
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 enterprise area: enterprise, audit log, organisation, SAML, SSO ✨ feature New feature or request size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants