Uncaught exception when canceling a request while sending body

[pmdartus]

Observation

Mockttp throws the following uncaught exception when canceling a client HTTP request while sending body:

Error: Aborted
    at IncomingMessage.<anonymous> (/Users/p.dartus/code/_tmp/failed-post/node_modules/mockttp/dist/util/buffer-utils.js:63:40)
    at Object.onceWrapper (node:events:627:28)
    at IncomingMessage.emit (node:events:525:35)
    at IncomingMessage._destroy (node:_http_incoming:224:10)
    at _destroy (node:internal/streams/destroy:109:10)
    at IncomingMessage.destroy (node:internal/streams/destroy:71:5)
    at abortIncoming (node:_http_server:747:9)
    at socketOnClose (node:_http_server:741:3)
    at Socket.emit (node:events:525:35)
    at TCP.<anonymous> (node:net:313:12)
node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

Repro steps:

import http from 'node:http';
import mockttp from 'mockttp';

const SERVER_PORT = 8080;
const PROXY_PORT = 8081;

const remote = mockttp.getLocal();

await remote.forAnyRequest().thenTimeout();
await remote.start(SERVER_PORT);

const server = mockttp.getLocal({
    debug: true
});

await server.forAnyRequest().thenPassThrough();
await server.start(PROXY_PORT);

const req = http.request({
    method: 'POST',
    host: 'localhost',
    path: `http://localhost:8080`,
    port: 8081,
    headers: {
        host: `localhost:8081`,
    },
});

req.write('Foo bar');

setTimeout(() => {
    req.on('error', () => {});
    req.destroy();
}, 1000);

Version

• Node: 14, 16, 18
• Mockttp: 3.6.1

[pmdartus]

After further inspection, the unhandled rejection originates from the RequestRule.handle:

mockttp/src/rules/requests/request-rule.ts

Line 84 in 62fb22a

.then(() => waitForCompletedRequest(req))

In cases where the waitForCompletedRequest rejects, the promise rejection isn't handled. This only occurs when recordTraffic is enabled.

Side note: I realized that mocha is incorrectly surfacing unhandled rejection. The handled rejections are currently silenced by mocha, which isn't ideal. There is a whole thread discussing this issue mochajs/mocha#2640. That said it's unclear to me whether or not the mocha team will attempt to fix this. In the meantime, I would recommend rethrowing the unhandled promise rejection to force tests to fail.

[pimterry]

In cases where the waitForCompletedRequest rejects, the promise rejection isn't handled. This only occurs when recordTraffic is enabled.

Good find! I think it would be reasonable to fix this by just catching the error while recording, and storing only the data we did receive in that case (dropping/truncating the body on a best-efforts basis). For request recording I think that's OK, we don't want/need to be too strict, and although this is an error it should give us a perfectly usable representation of received data, which might well be useful to have in tests.

If you're running into this yourself, do you want to take a look? Covering this explicitly en route with a test for this case would be helpful too.

I would recommend rethrowing the unhandled promise rejection to force tests to fail.

Sure, I'd be happy with that as an addition to the test setup, seems like a good idea 👍. I'm sure I've seen issues with failed unhandled rejections with Mocha elsewhere though, where that was failing some of Mockttp's tests... But I can't confirm that, and it would certainly be good to explicitly ensure that that happens.

[pmdartus]

If you're running into this yourself, do you want to take a look? Covering this explicitly en route with a test for this case would be helpful too.

Yes, I am on it.