ClientRequest .abort() and ECONNRESET

Continuing the discussion from #32182 (comment)

The current assumption is that always 'error' or 'response' is emitted on the socket.

However, this is not always the case. Consider:

const req = http.get(someUrl);
req.abort();
req.on('error', common.mustCall()); // Fails

This will fail because the case of calling req.abort() before 'socket' will actually result in no error. Whether we get an error or not is a question of timing.

The current semantics are:

If abort() before req.'socket', emit req.'close'.
If abort() after req.'socket' but before req.'response', emit req.'error'.
If abort() after req.'response', emit req.'close' & res.'aborted'.

My main concern (which breaks the above assumption) is the first scenario here. However, I noticed that fixing this will cause lots of other tests to fail.

What can/should we do about this? At a minimum I'd like to clarify the docs.

@mcollina

@nodejs/http @mcollina @lpinca

My main concern (which breaks the above assumption) is the first scenario here. However, I noticed that fixing this will cause lots of other tests to fail.

Which test will fail?

Which test will fail?

e.g.

// test/parallel/test-http-abort-before-end.js
'use strict';
const common = require('../common');
const http = require('http');

const server = http.createServer(common.mustNotCall());

server.listen(0, common.mustCall(() => {
  const req = http.request({
    method: 'GET',
    host: '127.0.0.1',
    port: server.address().port
  });

  req.on('abort', common.mustCall(() => {
    server.close();
  }));

  // This should error?
  req.on('error', common.mustNotCall());

  req.abort();
  req.end();
}));

Full list here https://gist.github.com/ronag/379a377e01eb2dfe43c62d54c0220c1a

I actually think the current behavior is fine and it's the assumption that is incorrect. Aborting before the socket is created should not be considered an error condition.

@jasnell

@jasnell: What is your opinion on abort() and destroy() behaving differently in this regards? i.e. with destroy() it is an error condition.

http: doc deprecate abort and improve docs

Navigation Menu

Repository navigation

ClientRequest `.abort()` and `ECONNRESET` #32225

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ClientRequest .abort() and ECONNRESET #32225

Description

Activity

ronag commented on Mar 12, 2020

mcollina commented on Mar 12, 2020

ronag commented on Mar 13, 2020

jasnell commented on Mar 13, 2020

ronag commented on Mar 13, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

ClientRequest `.abort()` and `ECONNRESET` #32225