fb55 / node-minreq

minimalistic request library for node
BSD 2-Clause "Simplified" License
8 stars 1 forks source link

Error in production #4

Open altano opened 10 years ago

altano commented 10 years ago

Hey there,

So I'm using minreq/cornet to power the latest version of Bookcision, a bookmarklet that people run on the Kindle website to help them download their notes/highlights.

I've instrumented the bookmarklet with exception reporting, courtesy of the excellent Rollbar.com, and the most commonly hit exception for me is:

TypeError: undefined is not a function

1  File "webpack-module://./~/minreq/index.js" line 242 col 1 in [anonymous]

which is this line:

if(!("timeout" in options) || options.timeout){
    this._reqTimeout = setTimeout(function(){
        if(!scope._ended){
            scope._request.abort();   <===============

            scope.emit("timeout");
            scope.emit("error", Error("ETIMEDOUT"));
        }
    }, options.timeout || 1e4);
}

I looked at the code for a while and can't tell what could cause this. Amazon frequently returns 500 errors AND it's very possible users are hitting the timeout of 10 seconds, but I don't see what would cause scope._request.abort to not be a function.

Any insights?

fb55 commented 10 years ago

That's just awful. My guess is that this happens when no socket has been assigned yet.

I'm not very happy with minreq in it's current form – it lacks tests, doesn't live up to the promise of supporting custom protocols (the initial idea was to have a simple API for adding support for eg. ftp or magnet protocols) and ignores a couple of edge-cases.

For now, request is probably the better choice - it's reliable, well tested and has a great number of contributors.

altano commented 10 years ago

It's not that awful, don't worry: We don't have that many users. :)

And htmlparser2/cornet are working wonderful to provide really clean, in-browser screen scraping that was absolutely to trivial to write. What a ridiculous layer though, bundling an html parser into the browser's JS interpreter.

Anyway, I'll switch to request asap.

Thanks, Felix.

fb55 commented 10 years ago

I'll keep this open, as it's a legit bug.