search

SpiderStrategies / node-tweet-stream

Node twitter module to hook into the public filter streaming, seamlessly updating the tracking keywords.
210 stars 43 forks source link

add explanation string and don't reconnect on 4xx status code #40

Closed robturtle closed 8 years ago

robturtle commented 8 years ago

As we can see in all these 4xx errors, it's all due to the user's fault who construct an illegal query. In such case no matter how many reconnections you tried, the result will still be the same. Thus I think make the stream abort and emit 'error' would be appropriate.

And when an 4xx error occur, providing some short explanation might be good for debugging.

nathanbowser commented 8 years ago

This looks good, but there's now a failing test.

nathanbowser commented 8 years ago

Oh, I see. Let's leave the reconnect event, but you can still embed the detailed error.

robturtle commented 8 years ago

Crap the Github didn't notify me about your comments and I just see it until now. I edited the changes and split them into 3 parts:

  1. add error explanations
  2. for all http errors, only reconnect on 503 since the stream can't be recovered from other situations; while other http errors will still emit 'reconnect'
  3. make other http errors emit 'error' instead and adjust test file accordingly

I believe the changing of reconnecting behavior is reasonable cause the users definitely don't want their wrong http requests keep retrying again and again until hit the rate limit.

nathanbowser commented 8 years ago

Agreed. Awesome. Thanks!

nathanbowser commented 8 years ago 
[koopa@dash-dev node-tweet-stream (master)]$ npm publish
+ node-tweet-stream@2.0.0

Bumped to major since it's breaking.