search

mattsse / chromiumoxide

Chrome Devtools Protocol rust API
Apache License 2.0
787 stars 81 forks source link

Include Chromium `stderr` when failing to launch #128

Closed demurgos closed 1 year ago

demurgos commented 1 year ago

This PR addresses the issue #124. It updates the code waiting for the browser to be ready to provide more context if an error occurs.

The PR is split into two commits:

  1. The first commit updates the lib to use the process utilities from either async-std or tokio instead of the standard library. The main goal of this is to get asynchronous I/O streams, in particular stderr. This is achieved by defining a wrapper type AsyncProcess abstracting the runtime selected by the compile-time features. Since the process can be accessed through a getter, this commit is a breaking change (type changed).
  2. The second commit refactors the ws_url_from_output function to improve error handling. In particular it now detects if the process crashes as soon as it starts (previously it would have waited until the timeout before detecting it), and returns stderr content on error. Returning stderr content is extremely helpful when troubleshooting issues.

I had an issue when running chromiumoxide in GitLab CI because it was running as root but I did not disable the zygote sandbox. Previously the library only returned "LaunchTimeout", but with my changes I could make it return "LaunchExit" with the standard error explaining that running as root requires to disable the sandbox. It allowed me to fix my issue more easily.

The error formatting uses the same logic as the standard library: try to print as a UTF-8 string, with a fallback to bytes on error.

demurgos commented 1 year ago

The Clippy errors detected by CI are not related to my changes. Should I fix them?

mattsse commented 1 year ago

I think the breaking change is fine as well.

mattsse commented 1 year ago

The Clippy errors detected by CI are not related to my changes. Should I fix them?

I believe they are unrelated to this change and the result of some code gen.

I haven't gotten around to fixing them yet, but I would gladly accept a PR :)

demurgos commented 1 year ago

I sent the Clippy errors fix (and an error message fix) in https://github.com/mattsse/chromiumoxide/pull/129