search

dafortune / hapi-qs

Bring back qs support for hapi 12
10 stars 7 forks source link

Handle qs parsing errors #1

Closed hueniverse closed 8 years ago

hueniverse commented 8 years ago

Parsing can fail.

dafortune commented 8 years ago

Good catch! Thanks! Will add a try catch just in case. Anyway, could you point out any specific case that might cause a failure?

hueniverse commented 8 years ago

Just look at the qs code.

dafortune commented 8 years ago

Might be me but I don't see an obvious point of failure on qs code + it seems not to fail loudly to any of my (manual) tests, also don't see explicit error handling logic on body-parser [it might be up on the stack] (https://github.com/expressjs/body-parser/blob/master/lib/types/urlencoded.js#L216 + https://github.com/expressjs/body-parser/blob/master/lib/types/urlencoded.js#L129).

Anyway, I just gave it a quick look, will give it a better look tomorrow.

hueniverse commented 8 years ago

@nlf can qs fail?

nlf commented 8 years ago

it shouldn't be able to

dafortune commented 8 years ago

Thanks @nlf. Let's close this issue for now then. Will re-open in case we find a failure point.

Feel free to comment here if you think we need to add any kind of deeper error handling.

Thanks both! Happy holidays!

dafortune commented 8 years ago

Just for future reference, here a posible error handling code https://github.com/daf-spr/hapi-qs/pull/2