Skip to content

feat: Guard against unbounded synchronous promise chain recursion#527

Merged
nev21 merged 2 commits into
mainfrom
nev21/SyncDepth
Jul 8, 2026
Merged

feat: Guard against unbounded synchronous promise chain recursion#527
nev21 merged 2 commits into
mainfrom
nev21/SyncDepth

Conversation

@nev21

@nev21 nev21 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Resolving a very deep or "hostile" self-resolving thenable through createSyncPromise could grow the call stack without bound and overflow it. syncItemProcessor now tracks consecutive synchronous continuations and forces a microtask hop once a configurable depth setMaxSyncPromiseChainDepth, default 200) is exceeded, letting the stack unwind before continuing.

Copilot AI review requested due to automatic review settings July 8, 2026 03:57
@nev21 nev21 requested review from a team as code owners July 8, 2026 03:57
@nev21 nev21 added this to the 0.6.1 milestone Jul 8, 2026
@nev21 nev21 modified the milestones: 0.6.1, 0.7.0 Jul 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a safety guard to the synchronous promise reaction processor so deeply (or recursively) chained synchronous continuations can’t grow the call stack without bound, by forcing a microtask hop once a configurable chain depth is exceeded.

Changes:

  • Introduces a configurable max synchronous chain depth and enforces it in syncItemProcessor via _runPendingItem.
  • Exposes the new configuration API (setMaxSyncPromiseChainDepth) from the package entrypoint.
  • Adds targeted tests covering shallow chains, very deep chains (no stack overflow), forced microtask hop behavior, and disabling/resetting the guard.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
lib/src/promise/itemProcessor.ts Adds depth tracking + guard logic and a new setter to configure the max synchronous chain depth.
lib/src/index.ts Re-exports setMaxSyncPromiseChainDepth as a public API.
lib/test/src/promise/syncChainDepth.test.ts Adds regression and behavior tests for deep/recursive sync promise chains and configuration options.

Comment thread lib/src/promise/itemProcessor.ts Outdated
Comment thread lib/src/promise/itemProcessor.ts Outdated
Comment thread lib/test/src/promise/syncChainDepth.test.ts
Comment thread lib/test/src/promise/syncChainDepth.test.ts Outdated
Comment thread lib/test/src/promise/syncChainDepth.test.ts
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.67442% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.55%. Comparing base (59999ea) to head (8f5cf13).

Files with missing lines Patch % Lines
lib/src/promise/itemProcessor.ts 97.67% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #527      +/-   ##
==========================================
- Coverage   97.57%   97.55%   -0.02%     
==========================================
  Files          28       28              
  Lines        1484     1516      +32     
  Branches      350      358       +8     
==========================================
+ Hits         1448     1479      +31     
- Misses         36       37       +1     
Files with missing lines Coverage Δ
lib/src/promise/itemProcessor.ts 98.66% <97.67%> (-1.34%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nev21 nev21 force-pushed the nev21/SyncDepth branch 2 times, most recently from c0f1ec3 to 762c310 Compare July 8, 2026 04:24
@nev21 nev21 requested a review from Copilot July 8, 2026 04:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • common/config/rush/npm-shrinkwrap.json: Generated file

Comment thread lib/src/promise/itemProcessor.ts
Comment thread lib/test/src/promise/syncChainDepth.test.ts Outdated
Comment thread common/config/rush/npm-shrinkwrap.json

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread lib/src/promise/itemProcessor.ts Outdated
Comment thread lib/src/promise/itemProcessor.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread lib/src/promise/itemProcessor.ts Outdated
@nev21 nev21 force-pushed the nev21/SyncDepth branch from 04f4cf8 to 96f2abe Compare July 8, 2026 06:24
Resolving a very deep or "hostile" self-resolving thenable through createSyncPromise could grow the call stack without bound and overflow it. syncItemProcessor now tracks consecutive synchronous continuations and forces a microtask hop once a configurable depth  setMaxSyncPromiseChainDepth, default 200) is exceeded, letting the stack unwind before continuing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread lib/src/promise/itemProcessor.ts
Comment thread lib/src/index.ts
Copilot finished work on behalf of nev21 July 8, 2026 06:51
@nev21 nev21 enabled auto-merge (squash) July 8, 2026 06:55

@nevware21-bot nevware21-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved by nevware21-bot

@nev21 nev21 merged commit c133f5a into main Jul 8, 2026
10 checks passed
@nev21 nev21 deleted the nev21/SyncDepth branch July 8, 2026 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants