search

panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Throwing with more details on error in `processResponse` #264

Closed alexgreenland closed 4 years ago

alexgreenland commented 4 years ago

In processResponse when it's a not a 200/OK response, is there anything against throwing with the response body even if it doesn't contain the property error? When this happens currently, I see a status code mismatch but no details.

https://github.com/panva/node-openid-client/blob/049e578ff513d092d7d21a9bd4211da9e2089614/lib/helpers/process_response.js#L31

I've seen quite a few providers return with a 400 in response to token grant but not include error; instead details are in other properties of the JSON, and it would be useful to throw the details in the body so we can see what went wrong. We know it's not a 200, so do we need to check if there's an error prop?

Thanks

panva commented 4 years ago

err.response is populated tho, isn't it? So you're free to inspect it anyway you want for the non-conform providers.

alexgreenland commented 4 years ago

Indeed, this works. Was just wondering about the rationale.

Incidentally, I've seen some providers return 200 with an error prop!

panva commented 4 years ago

Well, i can’t speak to nor build the client around bs behaviours ;)