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

puppeteer.launch({ pipe: true }) may crash due to unhandled stream errors #4374

Closed
justjake opened this issue Apr 30, 2019 · 3 comments · Fixed by #4705
Closed

puppeteer.launch({ pipe: true }) may crash due to unhandled stream errors #4374

justjake opened this issue Apr 30, 2019 · 3 comments · Fixed by #4705

Comments

@justjake
Copy link

justjake commented Apr 30, 2019

Steps to reproduce

Tell us about your environment:

  • Puppeteer version:
    • example for 1.14.0
    • same issue exists in 1.15.0, but repro is more lengthy; requires a separate bash script
  • Platform / OS version:
    • macOS 10.14.3 (18D42)
    • Debian 9 via docker image node:10.15.3-slim
  • URLs (if applicable):
  • Node.js version: v10.15.3

What steps will reproduce the problem?

Attempt to puppeteer.launch({ pipe: true }) with any condition that causes Chrome to crash on launch.

To demonstrate, I launch bash instead of Chrome, but you can also reproduce this using just puppeteer.launch({ pipe: true }) in a standard Docker container where Chrome can't use its sandbox. The behavior is very similar.

$ cat puppeteer-pipe-bug.js
const puppeteer = require("puppeteer")

async function main() {
        try {
                const browser = await puppeteer.launch({
                        pipe: true,
                        // This is not Google Chrome, but a misconfiguration in launch() still shouldn't
                        // cause unhandleable errors and crash the Node process
                        executablePath: "/bin/bash",
                        args: ["-c", "sleep 1; kill -9 $?"],
                })
                console.log("no puppeteer.launch() error")
                browser.close()
        } catch (err) {
                console.log("caught error", err)
        }
}

main()

What is the expected result?

puppeteer.launch() should either throw an error, or return a rejected promise in all error cases.

Ideal output:

$ node puppeteer-pipe-bug.js
caught error { Error: Protocol error (Target.setDiscoverTargets): Target closed.
    at Promise (/Users/jitl/src/notion/node_modules/puppeteer/lib/Connection.js:74:56)
    at new Promise (<anonymous>)
    at Connection.send (/Users/jitl/src/notion/node_modules/puppeteer/lib/Connection.js:73:12)
    at Function.create (/Users/jitl/src/notion/node_modules/puppeteer/lib/Browser.js:34:22)
    at Launcher.launch (/Users/jitl/src/notion/node_modules/puppeteer/lib/Launcher.js:177:37)
  message: 'Protocol error (Target.setDiscoverTargets): Target closed.' }

What happens instead?

$ node puppeteer-pipe-bug.js
events.js:174
      throw er; // Unhandled 'error' event
      ^

Error: write EPIPE
    at WriteWrap.afterWrite [as oncomplete] (net.js:782:14)
Emitted 'error' event at:
    at onwriteError (_stream_writable.js:431:12)
    at onwrite (_stream_writable.js:456:5)
    at _destroy (internal/streams/destroy.js:40:7)
    at Socket._destroy (net.js:607:3)
    at Socket.destroy (internal/streams/destroy.js:32:8)
    at WriteWrap.afterWrite [as oncomplete] (net.js:784:10)
@justjake justjake changed the title puppeteer.launch({ pipe: true })` may crash due to unhandled stream errors puppeteer.launch({ pipe: true }) may crash due to unhandled stream errors May 1, 2019
@justjake
Copy link
Author

justjake commented May 1, 2019

Here's the workaround I'm using: master...justjake:master

@mnmkng
Copy link
Contributor

mnmkng commented Jul 9, 2019

👍 + 1 for being able to prevent pipe errors from crashing our process.

@mnmkng
Copy link
Contributor

mnmkng commented Jul 16, 2019

@aslushnikov Thank you.

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