An error should be thrown, not returned

[aleybovich]

At this following line: https://github.com/ethereumjs/keythereum/blob/e81b70e62dccc121683f494d283b7fde7679326e/index.js#L547

if (!filepath) {
    return new Error("could not find key file for address " + address);
}

I think the error should be thrown, not returned.

Created a pull request: https://github.com/ethereumjs/keythereum/pull/60

[aleybovich]

Actually, testing further - the pull request won't fix the issue fully. The error thrown from the async function inside importFromFile will cause the process to exit silently. The real fix woudl be changing the callback signature and returning both success and error in that callback, like cb(err, result) instead of cb(result). This woudl be a breaking change for any client already using keythereum, so I am curious what you would advice here? @tinybike

[tinybike]

I think that's a bug. Good catch! AFAIK returning from inside the async callback doesn't make sense (nor does throwing, as you pointed out). I think the fix is:

return cb(new Error("could not find key file for address " + address));

The caller has to then check:

keythereum.importFromFile(address, datadir, function (resultOrError) {
    if (resultOrError instanceOf Error) {
        // handle the error
    } else {
        // oh happy day!
    }
});

Agree 100% that ideally we'd replace cb(result) with the more standard cb(err, result). (I should have written it this way initially and I didn't because I'm a dolt.) The reason I haven't made this change is just because a number of people are already using this library and I didn't want to break things for them.