quic: destroy stream on invalid promise body

[christianaurichzm]

stream.setBody(Promise.resolve(42)) crashes the process with an unhandled rejection, while stream.setBody(42) throws ERR_INVALID_ARG_TYPE synchronously.

const stream = await clientSession.createBidirectionalStream();
stream.setBody(Promise.resolve(42));

configureOutbound() calls itself with the resolved value inside the fulfillment handler of PromisePrototypeThen(). The rejection handler only covers body itself rejecting, so when that call throws, the error goes to the promise returned by PromisePrototypeThen(), which nothing handles.

This wraps that call in a try/catch and destroys the stream with the error, the same way the rejection handler already does. The same goes for ERR_INVALID_STATE when the promise resolves to a FileHandle already in use by another stream. Nothing changes for a body that rejects.

On main the new test crashes. With this change all test/parallel/test-quic-*.mjs tests pass and make lint is clean.

[nodejs-github-bot]

Review requested:

• @nodejs/quic

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.37%. Comparing base (24b9ca9) to head (413eb7d).
⚠️ Report is 79 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66260      +/-   ##
==========================================
+ Coverage   90.30%   90.37%   +0.06%     
==========================================
  Files         789      790       +1     
  Lines      272880   273847     +967     
  Branches    52106    52382     +276     
==========================================
+ Hits       246423   247477    +1054     
+ Misses      16918    16858      -60     
+ Partials     9539     9512      -27     

Files with missing lines	Coverage Δ
lib/internal/quic/quic.js	100.00% <100.00%> (ø)

... and 84 files with indirect coverage changes

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

[pimterry]

Thanks @christianaurichzm! Good find, fix LGTM.

Would be nice to avoid the extra Then step imo, but I don't feel strongly.

[pimterry]

Nit: this adds another promise layer, which adds a microtask step and a small perf cost on every request, and we're only trying to catch sync errors so that layer's not really necessary.

Could we rewrite to use a try/catch inside the fulfilment step instead?

[christianaurichzm]

Good point, done. I switched it to a try/catch in the fulfillment handler, so there’s no extra promise and rejected body values are handled the same way as before.