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

Sometimes tweets are emitted as 'error' events #57

Closed
jbilcke opened this issue Jun 14, 2012 · 7 comments
Closed

Sometimes tweets are emitted as 'error' events #57

jbilcke opened this issue Jun 14, 2012 · 7 comments

Comments

@jbilcke
Copy link

jbilcke commented Jun 14, 2012

Sometimes I receive the tweets through the 'error' events emitted by ntwitter's stream object:

stream.on('error', function (err) {
if (err.text !== undefined) {
console.log("twitter error: "+inspect(err)); <-- a full, working tweet is printed here!
}
});

It's a bit random (it can happen or not, even if I don't touch my settings/code/tokens)
my guess it's some kind of http code misinterpreted by ntwitter as "error", but still valid? maybe a "url changed" or "redirection" http code?

I don't have much time right now to dig inside ntwitter internals and figure out what is happening, but if I can get more details I'll post them here.

my config: node 0.6 / ntwitter 0.3.0

@fabdrol
Copy link

fabdrol commented Jun 18, 2012

I am getting the same error, it seems to have to do with the parser, at line 43. There's a try..catch structure there, designed to emit an error when the parse failes...

Apparently, sometimes data isnt't formatted properly, which causes the parse to fail. Does anybody know if there's a more solid solution to this? It's something I've come across a lot in Javascript..

Parser.prototype.receive = function receive(buffer) {
  this.lastTime = new Date().getTime();
  this.buffer += buffer.toString('utf8');
  var index, json;

  // We have END?
  while ((index = this.buffer.indexOf(Parser.END)) > -1) {
    json = this.buffer.slice(0, index);
    this.buffer = this.buffer.slice(index + Parser.END_LENGTH);
    if (json.length > 0) {
      try {
        json = JSON.parse(json);
        this.emit('_data', json);
      } catch (error) {
        this.emit('error', json);
      }
    }
  }
};

@nicola
Copy link

nicola commented Jun 27, 2012

Same for me but I get

Error: Uncaught, unspecified 'error' event.
at EventEmitter.emit (events.js:50:15)
at ClientRequest. (/Users/io/Proj/feetshot/node_modules/ntwitter/lib/twitter.js:251:14)
at ClientRequest.emit (events.js:67:17)
at HTTPParser.onIncoming (http.js:1261:11)
at HTTPParser.onHeadersComplete (http.js:102:31)
at CleartextStream.ondata (http.js:1150:24)
at CleartextStream._push (tls.js:375:27)
at SecurePair.cycle (tls.js:734:20)
at EncryptedStream.write (tls.js:130:13)
at Socket.ondata (stream.js:38:26)

@timsavery
Copy link

+1...have been seeing this as well.

@SujayKrishna
Copy link

@nicolagreco I got the same error message.
I tried

stream.on('error', function(error){
    console.log(arguments);
  });

and i got { '0': 'http', '1': 406 }
406 is for Not Acceptable.
Wondering why i'm getting this error.
My Setup : Node 0.6.11 , ntwitter 0.2.8 on Windows 7.

@CrabBot
Copy link

CrabBot commented Aug 7, 2012

This is caused whenever you have an uncaught exception in your stream.on('data') callback. Humorously, that's why it appears so random. lol.

At any rate, here is the associated code:

      try {
        json = JSON.parse(json);
        this.emit('_data', json);
      } catch (error) {
        this.emit('error', json);
      }

The trycatch is no doubt meant to catch only the JSON.parse error, but do to the sychronous nature of EventEmitter.emit, it actually catching any error's in the user's stream.on('data') callback as well. You could handle this one of two ways,

by not emitting synchronously, which leaves most of the code the same:

      try {
        json = JSON.parse(json);
        process.nextTick(this.emit.bind(this, '_data', json));
      } catch (error) {
        this.emit('error', json);
      }

or by only catching the parse error and additionally, returning the actual error, or a new type of error:

      try { json = JSON.parse(json); } catch (error) {
        this.emit('error', new Error('Invalid JSON returned - '+error.message);
        // You should always throw error objects, not strings,
        // but alternatively as it was before...
        // this.emit('error', json);
      }
      this.emit('_data', json);     

@fent
Copy link

fent commented Aug 7, 2012

Nice find @CrabBot. I had actually found this a while ago but I did not realize it was related to this issue.

The solution I went with was the first one, process.nextTick().

@CrabBot
Copy link

CrabBot commented Aug 7, 2012

I submitted a pull request to resolve this. Since the try/catch is only there to catch the JSON.parse(), I prefer to limit it to the line in general to avoid situations like this. We don't really need process.nextTick() since we've already asynced since the initial call.

Additionally, since it causes problems for users/libraries/stack traces/situations like this when you don't, I throw a real error.

#65

Cheers!

@AvianFlu AvianFlu closed this as completed Aug 7, 2012
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

No branches or pull requests

8 participants