Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unusable error message when anything but 200 OK is returned #50

Closed
TilBlechschmidt opened this issue May 7, 2021 · 9 comments · Fixed by #51
Closed

Unusable error message when anything but 200 OK is returned #50

TilBlechschmidt opened this issue May 7, 2021 · 9 comments · Fixed by #51

Comments

@TilBlechschmidt
Copy link
Contributor

When the Selenium Endpoint does not return a valid JSON object but instead throws some kind of error (e.g. 502 Bad Gateway) that error code and the response body is not part of the thrown error. Instead, it contains null:

The WebDriver server returned an unrecognised response: Server returned unknown response: null

Here is the corresponding HTTP response captured with Wireshark:

HTTP/1.1 502 Bad Gateway
Content-Length: 148
Date: Fri, 07 May 2021 07:42:20 GMT
Content-Type: text/plain; charset=utf-8

Unable to forward request: error trying to connect: tcp connect error: Operation timed out (os error 110)

It would be neat if that error message and code would be propagated instead of discarded 🙂

@TilBlechschmidt
Copy link
Contributor Author

It looks like this problem originates in this method. It takes a status code and a deserialised JSON value. However, since the response body is not a JSON object, it can not be deserialised and hence null is returned.

The minimal change I'd propose is to change line 178 to also return the status code. The optimal change would be to change it so that the non-deserialised body content is passed to the function (or alternatively, just squeeze the raw body into a serde_json::Value of type String and pass that to the function in case the deserialisation fails).

@stevepryde
Copy link
Owner

So the WebDriverError::UnknownResponse variant could be modified to include status code and the body (as text?). Would that work for you?

@TilBlechschmidt
Copy link
Contributor Author

Yeah that'd be the preferred solution! That way it would be possible to debug errors where something went horribly wrong and you don't get JSON back 😉

@stevepryde
Copy link
Owner

Ok I might put up a PR and get you to check it. Give me a few minutes

@stevepryde
Copy link
Owner

#51

@stevepryde stevepryde linked a pull request May 7, 2021 that will close this issue
@TilBlechschmidt
Copy link
Contributor Author

Works as expected (see PR)

@TilBlechschmidt
Copy link
Contributor Author

Thank you for the very quick fix! 😊

@stevepryde
Copy link
Owner

Haha all good - I just happened to check my notifications after you sent it. Easy fix (and something I've wanted as well). Thanks for pointing out where the change needed to go. That saved a lot of time.

I'll publish this version now

@stevepryde
Copy link
Owner

v0.24.0 is live :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants