remcohaszing / rehype-mermaid

A rehype plugin to render mermaid diagrams
MIT License
80 stars 8 forks source link

Bug/Performance: "... browser has been closed" + Suggestion #11

Closed axe312ger closed 8 months ago

axe312ger commented 8 months ago

In my configuration, this plugin is triggered numerous times. It appears to launch and terminate a browser instance with each SVG creation, causing instability in my current deployments.

Occasionally, one browser crashes with the error browser.newPage: Target page, context or browser has been closed. Both Chromium and Firefox have similar issues.

To mitigate this, I've added a retry mechanism with brief pauses around the unified processor/transformation, which somewhat addresses the problem. However, the frequent opening and closing of the browser could still hinder performance.

Proposal: Would you be open to modifying the plugin to include an optional setting that allows for:

  1. Opening the browser once it's first activated/used.
  2. Keeping the browser open until the Node process ends or a specific "close/kill" browser command is issued.

This approach would enable maintaining a single Chrome/Firefox instance on Netlify, generating all SVGS, and closing it once Gatsby completes the build.

I'm up for assisting in the implementation and could submit a PR. :) Keen to hear your thoughts first, @remcohaszing.

Also, a big thanks for your plugin!

axe312ger commented 8 months ago

I realized I should have opened this probably in https://github.com/remcohaszing/mermaid-isomorphic 🙈

remcohaszing commented 8 months ago

I maintain both packages and they are closely related. I’m ok with tracking this here. :)

This plugin opens and caches the browser once per unified processor. The browser is opened if a file is being processed. It’s then kept open until it’s no longer used. One browser page is created per file, or one additionally if you use responsive dark mode.

There appears to be a flaw in this caching logic, though I haven’t encountered this myself yet.

I have a hunch the issue goes away if you either:

Could you try those potential solutions in your node_modules and see if it helps?

I’m not too keen on keeping the browser open for too long, but if needed, I’m not dismissing a solution in that direction either.

axe312ger commented 8 months ago

Thanks for the fast reply. Will try to monkey-patch or release a fork for the swap. Its not really happening locally, but kinda often on Netlify 🔍

remcohaszing commented 8 months ago

You can use pnpm patch or patch-package for that.

I would like to verify what the problem is before making actual changes.

remcohaszing commented 8 months ago

Have you had a chance to see if my proposed solution works? I’m pretty eager to fix this, but I haven’t been able to reproduce this issue myself yet.

axe312ger commented 8 months ago

Not yet, but I have it on my list and definitely want to check it out!

remcohaszing commented 8 months ago

This should be fixed by mermaid-isomorphic 2.1.2. Please let me know if the issue persists.

axe312ger commented 8 months ago

Omg nice, thanks! I'll deploy it right away :)

axe312ger commented 8 months ago

First deploy did not yield any broken attempts to generate the svgs... As this is flaky, I'll keep an eye on it.

Had to add merdmaid-isomorphic 2.1.2 to my resolutions in case someone else wants to get an early fix before this is released into rehype-mermaid :)

remcohaszing commented 8 months ago

You don't need resolutions/overrides or a rehype-mermaid release. It's semver patch. Just update your lockfile.