sindresorhus
Expected a Promise rejection for option errors
Expected behavior
A Promise rejection.
Current behavior
Errors are thrown directly.
Code to reproduce
ky('/', {
prefixUrl: 'https://example.com/'
});
It does: it returns fetch(), which returns a Promise which resolves with a Body.
I see only downsides to returning validation errors as rejections (including but not limited to the fact that some environments swallow unhandled rejections by default). I can't think of any practical reason to do so. That's just my two cents.
Benefits of throwing synchronously for input validation:
- Shorter syntax for us
- It is technically a bit faster (no waiting for the microtask queue)
- By distinguishing these from operational errors, it becomes harder for the user to ignore or not notice them (important to me because these are fatal programmer errors).
- Causes the program to actually crash, as expected. Avoids the problem of some environments sending promise rejections into the abyss. (See here and here, among others.)
I'm not trying to say it's a huge deal. But to me, throwing synchronously seems obviously superior.
The best counter argument I can come up with is that rejecting instead of throwing gives us some squishy notion of consistency that might be nice for users who do want to ignore these errors for some reason. But I can't imagine a scenario when someone would want to ignore input validation errors - it would effectively just make Ky a no-op, so there's no point.
Shorter syntax for us
I'm rebasing the code (you can see the PR). The shorter one is Promise rejection.
It is technically a bit faster
That's true.
By distinguishing these from operational errors, it becomes harder for the user to ignore or not notice them Causes the program to actually crash, as expected.
It depends on your point of view. Unhandled rejections? No problem, use global catch.
@sindresorhus https://github.com/sindresorhus/ky/pull/127#discussion_r277155396 said:
The Promise spec says that Promise-returning functions should always reject, and never synchronously throw.
So, replying to:
The existing behavior is correct. ky() doesn't return a Promise.
I disagree. ky() does return a Promise. It's generated by fetch() and resolves with a Body.
Sorry, @szmarczak is correct. I was reading the docs:
Returns a Response object with Body methods added for convenience.
Which is incorrect. It should say:
Returns a Promise for a Response object with Body methods added for convenience.
@sholladay This should always be followed:
The Promise spec says that Promise-returning functions should always reject, and never synchronously throw.
I was only closing as I was fooled by the docs.
I haven't been able to find anything like that in ECMA-262, nor in Promises/A+. I might have missed it, but I'm skeptical they would make that choice as part of the spec.
FWIW, that is not a spec. And this is the only part that I disagree with:
Even argument validation errors are not OK.
Anyway, I had to make throw-rejects as a more general solution to get async programs to crash properly and it will handle this case for anyone who remembers to use it. I can live with it...
Anyway, I had to make throw-rejects as a more general solution to get async programs to crash properly and it will handle this case for anyone who remembers to use it. I can live with it...
Hehe. I have https://github.com/sindresorhus/hard-rejection I think the Node.js team plans to eventually make uncaught rejections a hard error.