Skip to content

fix(mock): settle the dispatch when an async reply data function rejects - #5762

Open
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:fix/mock-async-reply-reject
Open

fix(mock): settle the dispatch when an async reply data function rejects#5762
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:fix/mock-async-reply-reject

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

What

MockPool/MockAgent .reply(statusCode, dataFn) supports an async data function — the returned promise is already awaited in handleReply, and it's exercised in the existing suite (test/mock-agent.js:578, :3487). But that awaited promise had only a fulfilment handler: if the data function rejects, no onResponse* callback ever fires, so the dispatched request never settles (hang) and the rejection escapes as an unhandledRejection.

Fix

Add the rejection arm, mirroring the file's own conventions — the sibling reply-options-callback path already handles both fulfil and reject, and the sync path re-checks aborted:

return body.then(
  (newData) => handleReply(mockDispatches, { ..._response, data: newData }),
  (err) => {
    if (aborted) return
    deleteMockDispatch(mockDispatches, key)
    handler.onResponseError?.(controller, err)
  }
)

The request now rejects with the error, exactly one terminal callback, no unhandled rejection. Sync/options paths are untouched.

Test

test/mock-async-reply-reject.js: .reply(200, async () => { throw boom }) then a request, with a timeout guard and an unhandledRejection listener. RED on main (unhandled rejection; request never settles). GREEN with the fix (rejects with boom, data fn called once, no unhandled rejection).

@codecov-commenter

codecov-commenter commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.49%. Comparing base (17117f6) to head (67a405b).

Files with missing lines Patch % Lines
lib/mock/mock-utils.js 84.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5762      +/-   ##
==========================================
- Coverage   93.50%   93.49%   -0.01%     
==========================================
  Files         110      110              
  Lines       39072    39084      +12     
==========================================
+ Hits        36534    36543       +9     
- Misses       2538     2541       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

The data-function form of .reply(statusCode, data) already awaits a
returned promise and re-runs handleReply with the resolved body, but the
promise had no rejection handler. When the data function rejects, no
terminal onResponse* callback fires: the dispatched request never settles
(hang) and the rejection surfaces as an unhandledRejection.

Add a rejection arm that mirrors the reply-options-callback path: respect
an in-flight abort, then delete the mock dispatch and emit a single
onResponseError on the controller.

Signed-off-by: Paco Cartones <pacocartones@users.noreply.github.com>
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.

2 participants