Skip to content

Emit request errors after aborted requests#4580

Open
puneetdixit200 wants to merge 2 commits into
hapijs:masterfrom
puneetdixit200:fix-aborted-error-events
Open

Emit request errors after aborted requests#4580
puneetdixit200 wants to merge 2 commits into
hapijs:masterfrom
puneetdixit200:fix-aborted-error-events

Conversation

@puneetdixit200
Copy link
Copy Markdown

Summary:

  • emit the public request error event when a late 500 error is produced after an aborted request has already finalized
  • reuse the same error-response logging helper for normal finalize and completed-response paths
  • add a regression test for a handler that throws after the client disconnects

Tests:

  • npm_config_cache=/tmp/hapi-npm-cache npx lab -a @hapi/code -m 5000 test/request.js -g "emits request-error when handler fails after abort|does not fail on abort|emits request-error once"
  • source ~/.nvm/nvm.sh && nvm use 18.20.8 >/dev/null && npm_config_cache=/tmp/hapi-npm-cache npx lab -a @hapi/code -m 5000 test/request.js -g "emits request-error when handler fails after abort|does not fail on abort|emits request-error once"
  • git diff --check

Fixes #4567

@puneetdixit200 puneetdixit200 force-pushed the fix-aborted-error-events branch from 0014f64 to 6964cf6 Compare May 23, 2026 17:24
@damusix
Copy link
Copy Markdown
Contributor

damusix commented May 24, 2026

@puneetdixit200 Thanks for the contribution ... something I picked up doing some testing as an edge case: double request-error emit when disconnectStatusCode: 500

The fix works correctly with the default disconnectStatusCode (499), but double-emits when a route sets it to 500:

server.route({
    method: 'GET',
    path: '/',
    options: {
        response: { disconnectStatusCode: 500 }
    },
    handler: async (request) => {

        const result = await someSlowOperation();  // client disconnects during this
        return result;                               // then this throws
    }
});

What happens:

  1. Client disconnects → _finalize()_logErrorResponse() logs the abort Boom (500) — 1st emit
  2. Handler throws after finalize → _setResponse()_logErrorResponse() logs the handler error (500) — 2nd emit

With the default 499, step 1 skips logging (not a 500), so only step 2 fires. But disconnectStatusCode validates as any integer >= 400, so 500 is legal.

Reproduction test (currently fails against this PR — emitCount is 2):

it('does not double-emit request-error when disconnectStatusCode is 500', { retry: true }, async (flags) => {

    const server = Hapi.server({ debug: false });
    const team = new Teamwork.Team();
    let emitCount = 0;
    const errors = [];

    server.events.on({ name: 'request', channels: 'error' }, (request, event) => {

        emitCount++;
        errors.push(event.error.message);
    });

    server.route({
        method: 'GET',
        path: '/',
        handler: async (request) => {

            clientRequest.destroy();
            await Hoek.wait(10);
            team.attend();
            throw new Error('late failure');
        },
        options: {
            response: {
                disconnectStatusCode: 500
            }
        }
    });

    await server.start();
    flags.onCleanup = () => server.stop();

    const clientRequest = Http.request({
        hostname: 'localhost',
        port: server.info.port,
        method: 'GET'
    });

    clientRequest.on('error', Hoek.ignore);
    clientRequest.end();

    await team.work;
    await Hoek.wait(50);

    expect(emitCount).to.equal(1);
    expect(errors[0]).to.equal('late failure');

    await server.stop();
});

Perhaps, if we know aborted requests as 499, we could specifically check for it to prevent the double log?

This is a very niche case and I have no idea who configures their routes this way.

@damusix damusix requested a review from kanongil May 24, 2026 21:47
Skip hapi-generated disconnect Boom responses when logging internal 500 request errors, so late handler failures after an abort are still emitted once without also emitting the configured disconnect response.

Generated-by: OpenAI Codex

Signed-off-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com>
@puneetdixit200
Copy link
Copy Markdown
Author

Pushed 106f641d for the disconnectStatusCode: 500 double-emit case.

What changed:

  • hapi-generated disconnect Boom responses (Request aborted, Request close, Response error) are no longer logged as internal 500 request errors.
  • late handler failures after an abort still emit request-error once.
  • added the disconnectStatusCode: 500 regression test from the review case.

Verified locally:

  • npx lab -a @hapi/code -m 5000 test/request.js -g "handler fails after abort|disconnectStatusCode is 500"
  • npx lab -a @hapi/code -m 5000 test/request.js
  • npx eslint lib/request.js test/request.js
  • git diff --check

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.

Error-tracking software doesn't catch hapi application errors if an incoming request gets prematurely closed

2 participants