feat: Add Abort Controller - #40
Conversation
|
Hi there! Thanks for your first pull request! We'll review it shortly. While it's getting reviewed, feel free to join our Discord community for suggestions and chat. |
github-actions
Bot
added
Needs Review
docs
| return new Promise(resolve => setTimeout(resolve, ms)); | ||
| export function sleep(ms: number, signal?: AbortSignal | null): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| if (signal?.aborted) { |
There was a problem hiding this comment.
One small edge case: if signal.reason is undefined (which can happen in older environments or if aborted without a specific reason), falling back to a standard Error means err.name will just be 'Error'.
In the README.md, you suggest users check if (err.name === 'AbortError'), which would fail for this fallback case. Similarly in the call in src/index.ts
| breaker.recordSuccess(domain); | ||
| return response; | ||
| } catch (err) { | ||
| lastError = err; |
There was a problem hiding this comment.
What happens if the actual fetch() call throws an AbortError?
We need to make sure this catch block doesn't swallow the cancellation and accidentally retry the aborted request.
AryanSharma48
left a comment
AryanSharma48
left a comment
There was a problem hiding this comment.
Some nitpicks for certain edge cases, otherwise great implementation!
Updated error handling in sleep function to throw DOMException with 'AbortError' name when aborted.
Updated error handling for abort scenarios to match native fetch behavior.
2 participants
Description
Feature: Added native AbortController support.
Fix: Cancelling a request now immediately interrupts any ongoing backoff delay (sleep), preventing the app from hanging.
Logic: Added pre-flight abort checks before network requests and preserved the original AbortError objects when throwing.
Testing & Docs: Added a dedicated test suite (abort.test.ts) and updated the TypeScript package's README.md with usage examples.
Fixes #5 (issue number)
Type of Change
Please check the option that applies:
Checklist
Design & Parity
README.mdor general documentation where necessary.examplesorwebsiteto reflect these changes.Quality & Testing
cd sandbox && npm install && npm start) in a separate terminal before running the tests.npm install && npm run build && npm testinsidepackages/smooth-api-ts) and all tests passed.pip install -e ".[dev]" && pytestinsidepackages/smooth-api-py) and all tests passed.