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

Fix dumpio: true logs about #createBundleFallback #18264

Closed
timvandermeij opened this issue Jun 17, 2024 · 0 comments · Fixed by #18270
Closed

Fix dumpio: true logs about #createBundleFallback #18264

timvandermeij opened this issue Jun 17, 2024 · 0 comments · Fixed by #18270
Labels

Comments

@timvandermeij
Copy link
Contributor

timvandermeij commented Jun 17, 2024

In PR #18260 we enabled dumpio: true for Puppeteer. This option forwards the browser logs to Node's log stream so that we can see any warnings/errors that are logged in e.g. the web console. This surfaced a few interesting issues that we should fix before we enable that further due to the spammy nature of said logs.

This issue tracks the following often repeated log from http://54.241.84.105:8877/34fcd127eefeaf9/output.txt:

JavaScript error: http://127.0.0.1:41223/build/generic/web/viewer.mjs, line 3023: Error: Not implemented: #createBundleFallback

I have tracked the problem down to

yield this.#createBundleFallback(lang);
being triggered, which in turn triggers

pdf.js/web/genericl10n.js

Lines 110 to 112 in 06800cd

if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
throw new Error("Not implemented: #createBundleFallback");
}
and because we're in testing mode the exception is thrown.

Both code paths were introduced in 97c2ce9 but unfortunately I don't know enough about the L10n bundle handling to be able to tell for sure what the issue is. However, in debugging this I found that we in fact have a valid bundle for en-us just before at

yield bundle;
which made me wonder why we also try to create a fallback bundle for this language, and thought: are we perhaps missing a continue after the yield here? If I look at the situation before the patch, see 97c2ce9#diff-28e788fc43e67c197d60ffb3054ffb599f083341963c64d156e7a87a0ced98ceR78, we would always continue implicitly after the yield, but now it basically falls through to the fallback bundle creation.

This might very well be intended though, and this is the part I don't know. I'm also not sure why there is a dedicated testing path in this code, because if we fall through we'll always get here in testing mode too. @Snuffleupagus Do you perhaps have ideas for this, since you probably know this code a lot better than I do?

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