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

Silent crash on node 6 #31

Closed
scottnonnenberg opened this issue May 18, 2016 · 2 comments · Fixed by #32
Closed

Silent crash on node 6 #31

scottnonnenberg opened this issue May 18, 2016 · 2 comments · Fixed by #32

Comments

@scottnonnenberg
Copy link
Contributor

scottnonnenberg commented May 18, 2016

Thanks for this package. So useful! :0)

I downloaded it and tried it on my blog today, and it was giving me just four lines of output. Confused, I started looking at the code, and realized that it finished in the middle of enqueuing its first set of links. With a new try/catch in HtmlChecker/enqueueLink I found that the process was indeed silently crashing:

TypeError: source.hasOwnProperty is not a function
    at cloneObject (lib/internal/linkObj.js:239:14)
    at cloneObject (lib/internal/linkObj.js:245:18)
    at cloneObject (lib/internal/linkObj.js:245:18)
    at Function.linkObj.resolve (lib/internal/linkObj.js:168:64)
    at enqueueLink (lib/public/HtmlChecker.js:158:10)
    at lib/public/HtmlChecker.js:110:5
    at process._tickCallback (internal/process/next_tick.js:103:7)

On node 6.1.0 it throws this error, and on node 4.4.3 it works properly. Seems that there are two bugs here - one about the hasOwnProperty issue, and the other about silently crashing!

@scottnonnenberg
Copy link
Contributor Author

Figured it out - it's because the value returned by querystring.parse() is now created via Object.create(null) instead of inheriting from object: https://github.com/nodejs/node/wiki/Breaking-changes-between-v5-and-v6#querystring

Fixing this will also require changes in the urlobj repo: https://github.com/stevenvachon/urlobj

Researching now, may have a couple pull requests ready soon...

@scottnonnenberg
Copy link
Contributor Author

I was able to fix the silent error thing by installing bluebird as the replacement for global.Promise - it provides a key method: onPossiblyUnhandledRejection(). In bin/blc:

var Bluebird = require('bluebird');
global.Promise = Bluebird;

Bluebird.onPossiblyUnhandledRejection(function(error){
  throw error;
});

Worth thinking about. Sadly, this method is not available in the standard promise implementation.

Either that, or get really rigorous always returning rejected promises up the stack. Lines 97-118 in HtmlChecker.js are a good example: a call().then() with no catch and no return. Gotta either return the promise up the stack or handle the error right there. Otherwise errors will be swallowed.

Here's something I wrote about promises: https://blog.scottnonnenberg.com/the-trouble-with-promises/

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

Successfully merging a pull request may close this issue.

1 participant