1562043 - Broken formatting of webdriver commands and responses in the log file

Status: RESOLVED FIXED

(Testing :: geckodriver, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Since bug 1396824 has been landed there is a broken log output format for the webdriver crate:

Before: POST /session/d6f90a56-e378-4c58-83e7-7a037ee624a1/alert/dismiss {}
After: `POST /session/{sessionId}/alert/dismiss Ok("{}")

As it looks like it's not correctly formatted, given that no actual sessionId is filled in, and there is an extra Ok() wrapper for the body. Get requests are similar with Ok("").

We should fix that for the 0.25.0 release.

Bastian, mind having a look?

Comment 1

[Bastien Orivel [:eijebong]]

So I think the easiest way to do this would be to add a warp::path::full() filter to the route building code and add it as a parameter to the built closure. This would give us the actual called path.
The other alternative would be to rebuild that path from the route we currently have and parameters but that seems way too complicated.

Any thoughts on that ?

Comment 2

[Andreas Tolfsen ❲:ato❳]

I don’t have an opinion on this on the count that I don’t know warp
intimately.

Passing along some additional data (at the expense of memory) instead
of reconstructing it (at the expense of computation) seems reasonable
to me, if I understand the constraints you’re concerned about.

Comment 3

[Bastien Orivel [:eijebong]]

Attachment: Bug 1562043 - Fix a debug statement when processing webdriver requests. r?whimboo

Since we moved to warp, this debug statement was printing the
unprocessed path (i.e /session/{sessionId}) because the closure didn't
know about the real path that was called (it only knew about the route
that was triggered and the parameters that were passed).

I added a warp filter to get the unprocessed full path to be able to
make logging useful again. This also fixes the body which had an extra
Result around it when it was debugged.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45103a964331
Fix a debug statement when processing webdriver requests. r=ato

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/45103a964331

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta for Fx69?

Comment 7

[Andreas Tolfsen ❲:ato❳]

That shouldn’t be necessary since this code isn’t part of Firefox,
but instead ships as separate binary.