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

bug: Node freezes when using the programmatic API #146

Closed
Brainy0207 opened this issue Dec 15, 2022 · 4 comments · Fixed by #148
Closed

bug: Node freezes when using the programmatic API #146

Brainy0207 opened this issue Dec 15, 2022 · 4 comments · Fixed by #148
Labels

Comments

@Brainy0207
Copy link

Context:

  • version (md-to-pdf -v): 5.2.0
  • platform (Unix, macOS, Windows): Windows
  • node version: 19.2.0

Describe the bug:

I have a really simple script:

const { mdToPdf } = require('md-to-pdf');

async function convertAsync() {
  const pdf = await mdToPdf({ content: '# Hello, World' }, {dest: 'output.pdf'});
  console.log(pdf.filename);
}

convertAsync();

If I run this script with node, it logs the filename but freezes after that. I have to press Ctrl+C to get it to return.

It seems that multiple chrome.exe processes are created in the background and node is waiting for them to exit. If I kill these processes with the task manager, node returns.

This seems to be related to #141.
The browser instance created in generateOutput is never closed for the programmatic API:

if (!browserPromise) {
browserPromise = puppeteer.launch({ devtools: config.devtools, ...config.launch_options });
}

And because closeBrowser is not exported to the top level, there seems to be no way to close this browser instance programmaticaly.

@Brainy0207 Brainy0207 added the bug label Dec 15, 2022
@simonhaenisch
Copy link
Owner

Thanks a lot for the bug report and digging into it, yeah I must have missed that 🙈

I'll see that I get it fixed soon.

simonhaenisch added a commit that referenced this issue Dec 17, 2022
Not really done properly because there's likely some edge cases where this implementation would fail wrt timing.

Fixes #146.
simonhaenisch added a commit that referenced this issue Dec 17, 2022
Not really done properly because there's likely some edge cases where this implementation would fail wrt timing.

Fixes #146.
@simonhaenisch
Copy link
Owner

simonhaenisch commented Dec 17, 2022

Turns out fixing this is a lot harder than I thought, mainly because of how the library behaves now when running multiple programmatic mdToPdf calls simultaneously (which happens also when running the tests).

There's two options to fix this now:

At the cost of performance, I think I'd go with the second solution, and rather fix the performance problem in a better way (I've been planning to change the programmatic API to accept an array input anyway).

@simonhaenisch
Copy link
Owner

@Brainy0207 should be fixed in 5.2.1, if you could verify that? Thanks!

@Brainy0207
Copy link
Author

I ran my test script, no freezing, so I guess it is fixed. Thanks for the quick response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants