Skip to content

fix: waitUntil Set leak and NaN port bypass in core utils#225

Merged
pi0 merged 1 commit into
mainfrom
fix/waituntil-leak-port-nan
Jul 14, 2026
Merged

fix: waitUntil Set leak and NaN port bypass in core utils#225
pi0 merged 1 commit into
mainfrom
fix/waituntil-leak-port-nan

Conversation

@pi0x

@pi0x pi0x commented Jul 14, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Two small core-utils fixes from the v1 stabilization review.

  • F4 — waitUntil leaked memory on every call. createWaitUntil added the chained .catch().finally() promise to the Set but the cleanup deleted the original promise (which was never added), so the Set grew unbounded on any long-running server. Now the chained promise is captured in a variable and that same promise is deleted on settlement.
  • F36 — NaN bypassed the port range check. NaN < 0 and NaN > 65535 are both false, so PORT=abc / PORT="" slipped past the guard and failed later with an obscure listen error. Added a Number.isNaN guard that throws a clear RangeError.

Added test/utils.test.ts covering both (leak-free after N resolved/rejected waitUntil calls, and RangeError for non-numeric/out-of-range ports).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved port validation to reject invalid, non-numeric, and out-of-range values with clearer errors.
    • Fixed promise tracking so completed or rejected tasks are properly removed.
    • Non-promise values no longer affect pending task tracking.

  • Tests

    • Added coverage for port parsing, validation, and wait-task cleanup behavior.

@pi0 @claude
fix: prevent waitUntil leak and reject NaN ports
8c3dd8e 
- F4: capture the chained promise so the finally cleanup deletes the
  entry that was actually added to the Set (the original promise was
  never added), preventing unbounded Set growth per waitUntil() call.
- F36: reject NaN ports (e.g. PORT=abc or PORT="") with a clear
  RangeError instead of an obscure downstream listen error.

Co-Authored-By: Claude Fable 5 <[email protected]>
@pi0x
pi0x requested a review from pi0 as a code owner July 14, 2026 11:18
@coderabbitai

coderabbitai Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8a804a9e-9625-43f1-98f3-3a0682044558

📥 Commits

Reviewing files that changed from the base of the PR and between f2edab1 and 8c3dd8e.

📒 Files selected for processing (2)
  • src/_utils.ts
  • test/utils.test.ts
📝 Walkthrough

Walkthrough

The utilities now validate port values more explicitly and report original inputs in errors. createWaitUntil cleans up chained promises and exposes pending-count state for testing, with new tests covering both utilities.

Changes

Utility behavior updates

Layer / File(s) Summary
Port validation and error reporting
src/_utils.ts, test/utils.test.ts		 resolvePortAndHost rejects NaN and out-of-range ports, reports the original input in RangeError, and tests numeric, string, empty, non-numeric, and out-of-range values.
Wait-until promise cleanup
src/_utils.ts, test/utils.test.ts		 createWaitUntil removes chained promises after completion or rejection, exposes _size, and tests cleanup plus ignored non-thenables.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: pi0

Poem

I’m a rabbit with promises tucked in my burrow,
Now chained ones vanish—no lingering sorrow.
Ports stay in range, errors tell what they saw,
Tests hop through each rule with a soft little paw.
Sniff sniff—clean utilities, hooray!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main changes: fixing the waitUntil Set leak and NaN port validation in core utils.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/waituntil-leak-port-nan

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.

@pkg-pr-new

pkg-pr-new Bot commented Jul 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/srvx@225

commit: 8c3dd8e

@pi0
pi0 merged commit 14f815f into main Jul 14, 2026
15 checks passed
@pi0
pi0 deleted the fix/waituntil-leak-port-nan branch July 14, 2026 12:02
@pi0x pi0x mentioned this pull request Jul 14, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pi0 pi0 pi0 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pi0x @pi0