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

headless Puppeteer warning message shows #544

Closed
earlAchromatic opened this issue May 30, 2023 · 6 comments · Fixed by #555
Closed

headless Puppeteer warning message shows #544

earlAchromatic opened this issue May 30, 2023 · 6 comments · Fixed by #555

Comments

@earlAchromatic
Copy link

Describe the bug
Puppeteer warning message is logged which will then be shown when people use my app.

To Reproduce
Steps to reproduce the behavior:

  1. Run mermaid cli (as cli or node api also)

Expected behavior
No error messages shown. They come from puppeteer so quiet option does nothing.

Screenshots

  Puppeteer old Headless deprecation warning:
    In the near feature `headless: true` will default to the new Headless mode
    for Chrome instead of the old Headless implementation. For more
    information, please see https://developer.chrome.com/articles/new-headless/.
    Consider opting in early by passing `headless: "new"` to `puppeteer.launch()`
    If you encounter any bugs, please report them to https://github.com/puppeteer/puppeteer/issues/new/choose.

  • OS: macOS
@aloisklink
Copy link
Member

As a temporary measure, you can make a ./my-puppeteer-config.json file with {"headless": "new"} in it, and pass it to the CLI using --puppeteerConfigFile ./my-puppeteer-config.json.

Unfortunately, there's a pretty significant performance hit (my tests sometimes show a 10x slowdown!!), especially for PDF output, see puppeteer/puppeteer#10071.

It also seems a lot less stable compared to the default headless mode, in my experience ({"headless": "new"} seems to crash/freeze a lot more often).

As mermaid-cli is often used to make .pdf files, I'm not sure if it's worth changing the default puppeteer config for mermaid-cli, just to hide a warning. However, if your use-case doesn't export PDFs, or you don't mind the performance hit, you're welcome to switch over to using {"headless": "new"}!

You could also pin your app to only use puppeteer versions before v19.11.0, e.g. something like npm install puppeteer@19.10.0.

@jt-nti
Copy link

jt-nti commented Jun 22, 2023

Using --puppeteerConfigFile with a file containing {"headless": "old"} also gets rid of the warning.

I agree changing the default puppeteer config to use the new headless mode isn't worth the potential problems just to hide a warning, but I think explicitly setting the headless mode to old might be. Would that be possible?

@aloisklink
Copy link
Member

I've found another work around, you can set the PUPPETEER_DISABLE_HEADLESS_WARNING environment variable when running mermaid-cli.

Using --puppeteerConfigFile with a file containing {"headless": "old"} also gets rid of the warning.

I agree changing the default puppeteer config to use the new headless mode isn't worth the potential problems just to hide a warning, but I think explicitly setting the headless mode to old might be. Would that be possible?

Hmmm, it's a bit of a hack, since it doesn't comply with Puppeteer's TypeScript types, which only accepts boolean | "new" | undefined. However, it does work with Puppeteer v19.11.2.

I had a look at the Puppeteer source-code, and it looks like "old" isn't a special value. Instead, any value that is truthy, but is non-true (and not "new") will get rid of the warning.

I've made a PR (see #555). @jt-nti, since I adapted your idea, would you like to have a Co-authored-by credit, by me adding a Co-authored-by: James Taylor <3535067+jt-nti@users.noreply.github.com> git trailer to my commit's description?

@jt-nti
Copy link

jt-nti commented Jun 23, 2023

@aloisklink, always happy to be associated with a bit of a hack, thank you :)

@aloisklink
Copy link
Member

@aloisklink, always happy to be associated with a bit of a hack, thank you :)

Sorry, looks like that PR was merged before I had the chance to add you as a co-author. That's my fault, I should have left the PR as a draft until I got your reply :(

Well, on the plus side, at least you're now no longer associated with the // @ts-expect-error hack that I had to add to make TypeScript stop throwing error messages :)

@jt-nti
Copy link

jt-nti commented Jun 26, 2023

I would definitely never do anything as hacky as // @ts-expect-error! :)

Thanks for getting a fix in so quickly.

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.

3 participants