Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http.IncomingMessage aborted Event isn't the same as http.IncomingMessage close Event #43688

Open
UnlimitedBytes opened this issue Jul 5, 2022 · 4 comments
Labels
doc Issues and PRs related to the documentations.

Comments

@UnlimitedBytes
Copy link

UnlimitedBytes commented Jul 5, 2022

Affected URL(s)

https://nodejs.org/api/http.html#class-httpincomingmessage

Description of the problem

The "aborted" event is deprecated and states "Deprecated. Listen for 'close' event instead." which isn't related in any way.
The close event fires whenever the stream is CLOSED while the aborted event only fires if the request is ABORTED.

For exmaple a little HTTP-Server:

// server.js
const http = require('http')

const server = http.createServer((request, response) => {
    request.on('error', (error) => {
        console.error('> THERE WAS AN ERROR', error)
    })

    request.on('aborted', () => {
        console.log('> REQUEST GOT ABORTED')
    })

    request.on('close', () => {
        console.log('> REQUEST GOT CLOSED')
        response.end('CLOSED');
    })

    request.resume(); // CONSUME ALL DATA
});
server.on('listening', () => console.log('HTTP-Server is running.'));
server.listen(3100, '127.0.0.1');

And some simple curl requests to this http server:
curl -F file=somesmallfile.bin http://127.0.0.1:3100

REQUEST GOT CLOSED

curl -F file=somebigfile.bin --limit-rate 1K http://127.0.0.1:3100 and cancel CTRL+C

REQUEST GOT ABORTED
THERE WAS AN ERROR Error: aborted
   at connResetException (node:internal/errors:704:14)
   at abortIncoming (node:_http_server:680:17)        
   at socketOnClose (node:_http_server:674:3)
   at Socket.emit (node:events:549:35)
   at TCP.<anonymous> (node:net:747:14) {
 code: 'ECONNRESET'
}
REQUEST GOT CLOSED

As we can see the close event gets triggered in both cases while the abort event only gets triggered in the event of a user interruption. Please clearify this as close isn't an alternative for aborted as stated above.

@UnlimitedBytes UnlimitedBytes added the doc Issues and PRs related to the documentations. label Jul 5, 2022
@benjamingr
Copy link
Member

cc @dr-js @ronag

@climba03003
Copy link
Contributor

I think we can follow the deprecate document and check if it is aborted by request.readableEnded.

const http = require('http')

const server = http.createServer((request, response) => {
    request.on('error', (error) => {
        console.error('> THERE WAS AN ERROR', error)
    })

    request.on('close', () => {
        if(request.readableEnded) {
          console.log('> REQUEST GOT CLOSED')
          response.end('CLOSED');
        } else {
          console.log('> REQUEST GOT ABORTED')
        }
    })

    request.resume(); // CONSUME ALL DATA
});
server.on('listening', () => console.log('HTTP-Server is running.'));
server.listen(3100, '127.0.0.1');

@benjamingr
Copy link
Member

I assumed that the idea was "don't rely on this code that does cleanup should'nt care if it was closed or aborted" but I concede Robert's intent might have been to check .readableEnded or something similar

@ronag
Copy link
Member

ronag commented Jul 6, 2022

Yep, you should check in 'close' whether or not the stream we prematurely closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

4 participants