1413292 - Don't include Rust stack trace with errors

Status: RESOLVED FIXED

(Testing :: geckodriver, defect)

Description

[James Graham [:jgraham]]

This isn't very useful since any error we actually return is by-definition handled by the code, and we can probably get more information just by looking at the error type and string. At the same time, it runs the risk of confusing users into thinking there was a bug in the driver when actually it's perfectly normal handling of invalid input.

Comment 1

[Andreas Tolfsen ❲:ato❳]

I agree per our discussion earlier.

It is worth noting that this is also an issue with Marionette, where
a stacktrace is included irregardless of whether the error stems
from an actual programming error or as an “expected” WebDriver
error.

Comment 2

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

So is that a regression with the latest error stack changes, which were the reason we shipped 0.19.1?

Comment 3

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1413292 - Stop sending geckodriver backtrace with errors.

The Rust backtrace from geckodriver is  not useful since any error
we actually return is by-definition handled by the code, and we
can probably get more information just by looking at the error type
and string.

At the same time, it runs the risk of confusing users into thinking
there was a bug in the driver when actually it's perfectly normal
handling of invalid input.

Review commit: https://reviewboard.mozilla.org/r/195158/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195158/

Comment 4

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1413292 - Remove backtrace crate dependency from geckodriver.

Made this a separate commit from that which actually removes the
in-tree vendored dependencies to make it easier to review.

Review commit: https://reviewboard.mozilla.org/r/195160/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195160/

Comment 5

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1413292 - Drop backtrace et al. vendored crates.

Review commit: https://reviewboard.mozilla.org/r/195162/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195162/

Comment 6

[Andreas Tolfsen ❲:ato❳]

(In reply to Henrik Skupin (:whimboo) from comment #2)

> So is that a regression with the latest error stack changes, which
> were the reason we shipped 0.19.1?

We are quite intentionally including backtraces as substitutes for
Marionette stacktraces at the moment.  It was introduced as “a
feature” in 0.14.

I filed https://bugzil.la/1413326 for discussing whether to also
drop them from Marionette.

Comment 7

[James Graham [:jgraham]]

Comment on attachment 8923979 [details]
Bug 1413292 - Stop sending geckodriver backtrace with errors.

https://reviewboard.mozilla.org/r/195158/#review200422

Comment 8

[James Graham [:jgraham]]

Comment on attachment 8923980 [details]
Bug 1413292 - Remove backtrace crate dependency from geckodriver.

https://reviewboard.mozilla.org/r/195160/#review200426

Comment 9

[James Graham [:jgraham]]

Comment on attachment 8923981 [details]
Bug 1413292 - Drop backtrace et al. vendored crates.

https://reviewboard.mozilla.org/r/195162/#review200428

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b014ab0f907
Stop sending geckodriver backtrace with errors. r=jgraham
https://hg.mozilla.org/integration/autoland/rev/e2e4d0bdc5a6
Remove backtrace crate dependency from geckodriver. r=jgraham
https://hg.mozilla.org/integration/autoland/rev/51b6b04296ae
Drop backtrace et al. vendored crates. r=jgraham

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5b014ab0f907
https://hg.mozilla.org/mozilla-central/rev/e2e4d0bdc5a6
https://hg.mozilla.org/mozilla-central/rev/51b6b04296ae

Comment 12

[NIcholas DiPiazza]

So if this is the error prior to this fix? 

connection refused
Build info: version: '3.5.3', revision: 'a88d25fe6b', time: '2017-08-29T12:42:44.417Z'
System info: host: '59bb758fcbd0', ip: '172.17.0.2', os.name: 'Linux', os.arch: 'amd64', os.version: '4.4.0-1043-aws', java.version: '1.8.0_111'
Driver info: driver.version: RemoteWebDriver
remote stacktrace: stack backtrace:
   0:           0x4edb3c - backtrace::backtrace::trace::hc4bd56a2f176de7e
   1:           0x4edb72 - backtrace::capture::Backtrace::new::he3b2a15d39027c46
   2:           0x440ac8 - webdriver::error::WebDriverError::new::ha0fbd6d1a1131b43
   3:           0x447ba9 - geckodriver::marionette::MarionetteHandler::create_connection::hf0532ddb9e159684
   4:           0x428570 - <webdriver::server::Dispatcher<T, U>>::run::h2119c674d7b88193
   5:           0x4029b9 - std::sys_common::backtrace::__rust_begin_short_backtrace::h21d98a9ff86d4c25
   6:           0x40be65 - std::panicking::try::do_call::h5cff0c9b18cfdbba
   7:           0x5e6a6c - panic_unwind::__rust_maybe_catch_panic
                        at /checkout/src/libpanic_unwind/lib.rs:99
   8:           0x41eb22 - <F as alloc::boxed::FnBox<A>>::call_box::h413eb1d9d9f1c473
   9:           0x5df13b - alloc::boxed::{{impl}}::call_once<(),()>
                        at /checkout/src/liballoc/boxed.rs:692
                         - std::sys_common::thread::start_thread
                        at /checkout/src/libstd/sys_common/thread.rs:21
                         - std::sys::imp::thread::{{impl}}::new::thread_start
                        at /checkout/src/libstd/sys/unix/thread.rs:84

What is going to be the error after this fix? Is the only info useful here "connection refused"?

Comment 13

[Andreas Tolfsen ❲:ato❳]

Note that this change hasn’t made it into a geckodriver release yet since it’s blocking bug 1401129.

The stacktrace you quote will be gone when 0.20.0 is released.

Comment 14

[NIcholas DiPiazza]

:ato @ato 
ok. understood. but can you help me understand, does this error hide the actual cause of this problem? Because I have no idea why this create connection is failing. How can I get the cause of the error?

also posted this question on https://stackoverflow.com/questions/48195485/how-to-get-more-error-info-from-a-selenium-geckodriver-failure

Comment 15

[Andreas Tolfsen ❲:ato❳]

The cause of the issue is that geckodriver is unable to connect to
Firefox.  The stacktrace here is misleading because it isn’t an
implementation problem.

I suggest filing a new issue against geckodriver [1] if you can’t
resolve it.  I’m suspecting, perhaps, that you are using a Firefox
that is too old.

  [1] https://github.com/mozilla/geckodriver/issues/new

Comment 16

[NIcholas DiPiazza]

i'm on 57.0.1 with Gecko 0.19.1