fent / node-miniget

A small http(s) GET library.
MIT License
53 stars 17 forks source link

doDownload errors lead to program crash #45

Closed nfearnley closed 4 years ago

nfearnley commented 4 years ago

There is no try/catch in doDownload, so any thrown errors bubble up until they reach the node process handler, and will end up crashing the program. Here's an example of such a program:

"use strict";

const miniget = require("miniget")

async function crashMe() {
    try {
        var url = 1;
        var result = await miniget.promise(url);
        console.log(result);
    }
    catch (err) {
        console.err("GOT AN ERROR");
    }
}

crashMe();

The error in this instance occurs because the url is not a string.

I've put a try/catch around the miniget.promise() call, but it fails to catch the raised error inside doDownload, and is never triggered. Instead I get this output:

url.js:154
    throw new ERR_INVALID_ARG_TYPE('url', 'string', url);
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "url" argument must be of type string. Received type number
    at Url.parse (url.js:154:11)
    at Object.urlParse [as parse] (url.js:148:13)
    at doDownload (crashme\node_modules\miniget\dist\index.js:90:28)
    at process._tickCallback (internal/process/next_tick.js:61:11)
    at Function.Module.runMain (internal/modules/cjs/loader.js:834:11)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:623:3)
nfearnley commented 4 years ago

Oh, and to be specific, I was running into this issue when using ytdl-core. I've looked through the code and have no idea how it was sending a url of undefined to miniget, but it was somehow. If this bugfix could be added to a new version of ytdl-core, it would solve a lot of my problems.

fent commented 4 years ago

that issue was fixed a few days ago on ytdl-core, make sure you're on latest version