0

Consider the following HTTP client, following official example of the http module`:

var http = require('http');
http.get('http://127.0.0.1:12345/', function(response) {
    console.log('got response ', response.statusCode);
    var body = [];
    response.on('data', function(data) {
        console.log('got data: ', data);
    });
    response.on('end', function() {
        body = Buffer.concat(body).toString();
        console.log('end body = ', body, ' complete = ', response.complete);
        console.log('success!');
    });
}).on('error', function(err) {
    console.log('ClientRequest error: ' + err);
    console.log('error!');
});

Here I make an HTTP request, and report on three occasions: whenever I start receiving an HTTP response, whenever I get some HTTP body data, whenever the response is complete, and on all errors.

I expect this code to always complete (unless the connection hangs) with exactly one of success! or error! in logs. However, if the connect resets after the server has already sent some headers, I get both. Here is an example output:

got response  200
got data:  <Buffer 48 65 6c 6c 6f 0a>
ClientRequest error: Error: read ECONNRESET
error!
end body =  Hello
  complete =  true
success!

How to reproduce:

  1. nc -l -p 12345 to start a debug server.
  2. Run the client with node a.js. I use node v18.14.2 on Windows, but I saw the same behavior on Ubuntu 22.04 with node v12.22.9 as well.
  3. Type the following response, but do not abort nc just yet: 
    HTTP/1.1 200 OK

Hello
  4. Terminate the nc process with task manager (Windows) or kill the TCP connection with sudo tcpkill -i lo port 12345 and writing something else to nc (Linux).

It seems like Node both detects the error (hence the error callback is fired) and considers the HTTP request a success (hence to end callback is fired). In my original code that resulted in two instances of my application continuing instead of one which was very confusing.

How do I make sure exactly one of those is fired, and only on successful HTTP responses?

yeputons
  • 8,478
  • 34
  • 67
  • 1
    Just use a promise. Since it's a one-way state machine, it will only generate one outcome, whichever happens first. If you use a more capable http library such as `got()` or `axios()` or `node-fetch()`, they will handle all this for you (since they are all promise-based). – jfriend00 Mar 06 '23 at 22:01

1 Answers1

1

I suggest taking a look at the once() function which removes itself after being called, here is a link to it's reference - https://nodejs.org/api/events.html#emitteronceeventname-listener

Also, your code seems to be a get-event handler on a server but is behaving as though it is reading data from a server, and so I am not sure if you have confused the two here or if I am mistaken, however I would also recommend taking a look at using something like 'Node Fetch' that simplifies your program execution flow - https://www.npmjs.com/package/node-fetch

And looks something like this instead of the traditional event handlers:

import fetch from 'node-fetch';

const response = await fetch('https://github.com/');
const body = await response.text();

console.log(body);
Amrita Bithi
  • 11
  • 3
  • Yes, but in my case every callback is called once. My confusion is that both `error` and `end` are called, once each. – yeputons Mar 06 '23 at 20:56
  • I edited my answer and mention that, I think you may be trying to access the response object before it has been used and may want to be looking at the 'request' object instead, as the style of the event handler you have for get() looks like a traditional server-side listener event and not one that would be pulling data in over a stream by itself if that makes sense. Personally I never parse data myself and always use a library that handles the housekeeping for me for reading the full buffer in, it is just far easier and cleaner. The JSON libraries are also useful for reading data from URLS. – Amrita Bithi Mar 06 '23 at 21:24
  • yes, the `on('error')` is set on the request object, as it's the only object that can process initial errors (like DNS failure). But `end` callback is set on the response object. I was inspired by [this example](https://nodejs.org/api/http.html#httpgeturl-options-callback). I guess it's outdated/never meant to be the recommended way of making HTTP requests..? – yeputons Mar 06 '23 at 21:34
  • It actually seems that `node-fetch` does something similar inside. It rejects the promise [on `'error'` of the request object](https://github.com/node-fetch/node-fetch/blob/8ced5b941cf36d0d7e0c1017aa2a4abcb29ecd89/src/index.js#L108-L109), and resolves it [on `response`](https://github.com/node-fetch/node-fetch/blob/8ced5b941cf36d0d7e0c1017aa2a4abcb29ecd89/src/index.js#L366). If both happen, the first one (`response` in my case) takes priority. E.g. in your code all network errors are just suppressed if the client succeeded to receive headers, as it seems to me from debugging of `node-fetch`. – yeputons Mar 06 '23 at 21:37