Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Parse Error in http.js #1956

Closed
japj opened this Issue · 27 comments

10 participants

Jeroen Janssen Ben Noordhuis Mikeal Rogers windyrobin Siddharth Mahendraker Ștefan Rusu Daza Grohovaz jamespeerless Koichi Kobayashi Kilian Ciuffolo
Jeroen Janssen

Hi,

I am using nodejs 0.5.10 on Windows, using nano 0.8.4 to connect to a couchdb (1.0.2) database.

This night my program failed with the following notice:

node.js:202
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
Error: Parse Error
    at Socket.ondata (http.js:1116:24)
    at TCP.onread (net.js:314:27)

It is a bit difficult to relate, but around the same time the nodejs program threw an exception I see a '500' error in the couchdb logging. Maybe that is related?

Ben Noordhuis

Can you trace the request with tcpdump or wireshark?

Jeroen Janssen

I have not been able to reproduce it yet, but the couchdb server is running on centos 6 so if it is enough to set this up on that machine (provided I have enough instructions) then I can set that up and 'hope' it occurs again.

Ben Noordhuis

I suspect there is no real Node bug here but that instead it's some error condition on the server side, seeing that couchdb logs a 500.

If couchdb prematurely closes the connection, an error will bubble up on the client side that you should be able to catch if your request object has an error event listener.

But if you want to know exactly what is going on, you'll have to trace the traffic going over the wire.

Jeroen Janssen

The only 'strange' thing I saw was that http.js:1116:24 relates to:

var ret = parser.execute(d, start, end - start);

at https://github.com/joyent/node/blob/master/lib/http.js#L1136

and in https://github.com/joyent/node/blob/master/src/node_http_parser.cc#L466 it generates the exception object which it then returns (as far as I can tell)?.

How should this Parse Error exactly propogate from node_http_parser.cc to http.js and then to the client side?

Jeroen Janssen

btw, I'm using the nano couchdb library and from looking at the nano sources it seems errors should propogate from it to my client code, so I am not entirely sure why I didnt get an error in my client instead of the above exception.

Ben Noordhuis

How should this Parse Error exactly propogate from node_http_parser.cc to http.js and then to the client side?

socket.destroy(exception) is called on parse errors. That exception is emitted as an error event on the socket that the error listener in http.js in turn emits as an error event on the request object.

Mikeal Rogers

yeah, this error should emit on the http.ClientRequest object if you're using 0.5.4+, if not it's a bug.

windyrobin

@bnoordhuis
@mikeal

After some hard work, I finally could reproduce it
node v0.5.10

server.js

var http = require("http");

 var server = http.createServer(function(req, res){
   res.writeHead({"Content-Type" : "text/html"});
   res.end("hello,world\n");
});

server.listen(3458);

get.js

 var http = require("http");

 var options = {
   port: 3458,
   path: '/'
 };

 http.get(options, function(res) {
   console.log("Got response: " + res.statusCode);
 }).on('error', function(e) {
   console.log("Got error: " + e.message);
 });

then I execute some cmds :

$ node server.js &
[1] 13034
$ curl localhost:3458/
hello,world
$ node get.js 
Got error: Parse Error

Note that when I write the server and the client in the same file ,
it runs ok (this's why the test/test-http* couldn't detect the error)

parse.js

var http = require("http");

 var server = http.createServer(function(req, res){
   res.writeHead(200, {"Content-Type" : "text/html"});
   res.end("hello,world\n");
 });

 const PORT = 3458;
 var options = {
   port : PORT,
   path :'/'
 }

 server.listen(PORT , function(){
   http.get(options, function(){
     console.log("get a response");
   }).on("error", function(err){
     console.log(err.message);
   })
 })

the result

$node parse.js 
get a response

and I also found another issue which created by me
is caused by this serious bug!!!
#1958

Siddharth Mahendraker

On top of this, if you don't console.log() the error in get.js, node runs without crashing, even though it should crash on an unhandled error.

Ben Noordhuis

@windyrobin: You forget to pass the status code in the res.writeHead() call in server.js.

@siddMahen: There is no unhandled error here, your error listener is simply a no-op.

@japj: Can I close this issue?

windyrobin

@bnoordhuis

I'm so sorry , I'am dizzy on this problem...

Besides that ,could you please give some tips on how to make http-keep-alive requests,
I don't want want the agent.socket be destroied when there is no pending request ,then I could
reuse the socket again in some mill-seconds later...

Ben Noordhuis

@windyrobin You are mixing issues, stick to #1958.

Jeroen Janssen

@bnoordhuis I still don't understand why I am getting the error with this specific stracktrace.

As far as I can tell, nano uses mikeal's request library.

The request library will call the nano callback whenever an error occurs during http requests at https://github.com/mikeal/request/blob/master/main.js#L163

Nano will call my callback and will pass the error through at https://github.com/dscape/nano/blob/master/nano.js#L144

So as far as I can tell, everyone is properly trying to handle the error right?
Then why does it go wrong?

Ben Noordhuis

You know that as a rule we don't support third-party modules, right? Can you reproduce the problem with a plain Node test case?

Jeroen Janssen

@bnoordhuis the http_parser code surrounding error handling has changed a fair bit (comparing 0.4.12 and 0.5.10). I am still trying to figure out if that could be the cause of the problem.

What would help in future analysis of similar issues is if nodejs http_parser.cc would actually use the error information from the http_parser in the exception (i.e. use http_errno_name/http_errno_description to fill the exception information it returns when generating a "Parse Errror"). I expect that is on the todo list though?

Ben Noordhuis

What would help in future analysis of similar issues is if nodejs http_parser.cc would actually use the error information from the http_parser in the exception (i.e. use http_errno_name/http_errno_description to fill the exception information it returns when generating a "Parse Errror").

Sounds like a good idea.

I expect that is on the todo list though?

It is now. :-)

Ștefan Rusu

Have something like this in node v0.4.12:

node.js:134
        throw e; // process.nextTick error, or 'error' event on first tick
        ^
Error: Parse Error
    at Socket.ondata (http.js:1231:22)
    at Socket._onReadable (net.js:677:27)
    at IOWatcher.onReadable [as callback] (net.js:177:10)

Still have to dig in order to find the problematic URL, but it does look like a node bug. There is an error listener for the request, therefore is not PEBCAK.

Ștefan Rusu

Found the URL: http://www.dennisbabkin.com/php/download.php?go=ScrEncCtl&file=screncctrl.zip

It appears to be some sort of race condition, but I can't reproduce it with code handling this URL alone. With this URL alone, the error is properly passed to the error listener. I guess I should dig some more.

Daza Grohovaz

i have the same problem with the native parser
some ideas??

node version 0.6.0

SERVER ANWSER:
HTTP/1.0 200 Connection established\r\n\r\n

CODE:
socket.ondata = function(data, start, end) {
var ret = parser.execute(data, start, end - start); <<< here breaks...
if (ret instanceof Error) {
socket.destroy(ret);
}
}

ERROR:
node.js:201
throw e; // process.nextTick error, or 'error' event on first tick
^
Error: Parse Error
at Socket.ondata (/root/nodechain/lib/nodechain.js:293:22)
at TCP.onread (net.js:334:27)

Ben Noordhuis

@dazagrohovaz: You're on your own here, the HTTP parser API is not for public consumption. That said, I added a 'no response headers' test in 8f15582 and it passes.

jamespeerless

SaltwaterC - I am seeing the same issue as you. With that URL alone the error gets handled fine. If I am processing a lot of urls I'll eventually hit one where this error is not captured. I cannot figure it out, been poking around for hours :-/

jamespeerless

In my case it looks like a bug with mikeal's request module. It looks like it's a race condition. It's a race between the error handler getting setup and the response returning (or emitting an error). Not entirely sure I am right but that's what it seems like. I'll update my issue on mikeal's request github.

Alternatively, what if the request emits an error before the ClientRequest object is returned so you could never have time to setup the error handler?

Mikeal Rogers

@bnoordhuis the parser is for public consumption as far as I know.

node-pcap uses it and I have a private project that also uses the parser directly. the API is very "close to the metal" and errors aren't expected to be as pretty as errors from http.js but AFAIK the http parser is a public API.

Ben Noordhuis

@mikeal: No, the HTTP parser API is private. It can and has changed and likely will again. I don't mind if you use it but don't expect it to be stable or even to stay around.

Koichi Kobayashi
Owner

Related to #2206?
When the response body is longer than Content-Length, the Parse Error occurs.
And if Keep-Alive is used, the error does not propagate to http.ClientRequest because the response is already completed (The error occurs after the response).

reproduce:

var http = require('http');
var data = '¥'; // U+00A5 (2 bytes [0xC2, 0xA5] in UTF-8)
http.createServer(function(req, res) { 
  res.writeHead(200, {
    'Content-Type': 'text/plain',
    'Content-Length': data.length // ERROR!! should use Buffer.byteLength()
  });
  res.end(data); // writes 2 bytes
}).listen(3000, function() {
  http.get({port: 3000}, function(res) { 
    console.log(res.headers);
    res.on('data', function(data) {
      console.log('DATA', data);
    }).on('end', function() {
      console.log('END');
    });
  });
});

result:

{ 'content-type': 'text/plain',
  'content-length': '1',
  connection: 'keep-alive' }
DATA <Buffer c2>
END

node.js:201
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
Error: Parse Error
    at Socket.ondata (http.js:1125:24)
    at TCP.onread (net.js:334:27)
Kilian Ciuffolo

Is it fixed? Having the same issue with large scraping tasks.
I am using @mikeal request module too.

Ben Noordhuis

@kilianc: The bug that @koichik's test demonstrates is fixed in master but not v0.6.

Ben Noordhuis bnoordhuis closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.