Return better error when doc._id is not decodable with decodeURIComponent#2545
Return better error when doc._id is not decodable with decodeURIComponent#2545jacargentina wants to merge 1 commit intoapache:masterfrom jacargentina:detect-malformed-id
Conversation
|
We don't ever just return an error, if you need to change the message you need to still rethrow it, it will be re-caught and the error will be returned currently all this does is return the error instead of a doc. Also this would be covered by invalid id so we don't need to make a new code, lastly we are trying towards always using native errors when possible so we probably want something more like try {
doc._id = decodeURIComponent(doc._id);
}
catch (e) {
var err = new TypeError('unable to decode id');
err.status = 400;
throw err;
} |
|
@calvinmetcalf I've tried to adapt my idea to "pouchdb structured code and programming style", by looking at how However, i had the option to include the try/catch on the very I think @nolanlawson should look at my patch first. |
|
so this is one of those parts of the code where you probably don't want to use to extrapolate style for the rest of the code as this is some dated code from when we didn't use JavaScript errors period, but what we try to do is pass async errors around but throw sync errors around. But that is secondary to us trying to avoid reusing custom errors like that because they won't have stack traces, like the one you used to debug this one here. |
|
I agree with Calvin about using a regular TypeError. About returning vs. throwing, I think you're getting tripped up by the fact that there's a coding error a few lines above where we return an error instead of throwing it. |
|
@jacargentina it is confusing so starting a patch to clear up the style |
|
@calvinmetcalf fixed that throwing error (b9d8644), so we should be able to throw instead of return now. |
|
@jacargentina Hah, good catch. None of us noticed that it made no sense to even decode the doc URI. :) #2652 fixes this. |
I've had a case where a doc._id was badly encoded (contained a % sign).
When doing db.sync(), the error was very generic; looking at the originating stack, the culprit was
decodeURIComponentThis commit fixes that, providing a new errors.MALFORMED_ID to handle that.