-
Notifications
You must be signed in to change notification settings - Fork 190
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
Setting up fake worker failed: "Failed to fetch dynamically imported module" #2554
Comments
It seems the relative path doesn't always work in your application. Can you find out why the duplicate In the meantime, there's an easy fix. pdfDefaultOptions.workerSrc = `/my-app/assets/pdf.worker-${getVersionSuffix(assetsUrl(pdfDefaultOptions.assetsFolder))}${
pdfDefaultOptions._internalFilenameSuffix
}.mjs`; in the constructor of your component or in your module. If you need to support older browsers as well, modify the lambda accordingly: pdfDefaultOptions.workerSrc = pdfDefaultOptions.needsES5
? `/my-app/assets/pdf.worker-${getVersionSuffix(assetsUrl(pdfDefaultOptions.assetsFolder))}-es5.mjs`
: `/my-app/assets/pdf.worker-${getVersionSuffix(assetsUrl(pdfDefaultOptions.assetsFolder))}${
pdfDefaultOptions._internalFilenameSuffix
}.mjs` I can't do that myself because some users have different folder layouts. |
It's not that it doesn't sometimes work in our application, but rather that the relative path to the worker javascript file is not correct. I added your suggested workaround in the following way pdfDefaultOptions.workerSrc = () => `./pdf.worker-${pdfjsVersion}${pdfDefaultOptions._internalFilenameSuffix}.mjs`; Since we don't use the bleeding edge version I skipped the getVersionSuffix function and since the "assets" part leads to the error, I also removed this. I'm not sure which part of the documentation points to the fact that I'll try explaining why the URI ends up having "assets/assets". In Or in other terms static get _setupFakeWorkerGlobal() {
return shadow(this, "_setupFakeWorkerGlobal", (async () => {
if (this.#zi)
return this.#zi;
return (await import(this.workerSrc)).WorkerMessageHandler // <= this is the dynamic import
}
)())
} By setting |
Small update In const e = new Worker(this.#Ui(t),{
type: "module"
})
// Where the function#UI
#Ui(t) {
return window.trustedTypes ? window.pdfViewerSanitizer.createScriptURL(t) : t
} So in the Important EDIT: I realized I misunderstood the |
Hi guys, |
@yoenispantoja Yes. Only, your issue isn't related to the issue of this thread. :) And if you say "my server doesn't allow *.mjs files", you make it sound like you can't change that. That'd be bad, because then your only choice would be to use an old version of the viewer. I think version 19 was the last version using standard *.js files, but I'm not sure. I tried to revert the migration to mjs files, but I failed. When the pdf.js team embraced the mjs format, they did so thoroughly. There's no way back. So you have to configure your server. Basically, all you have to do is telling your server to use the correct MIME type. I've described the configuration for several popular servers here: https://pdfviewer.net/extended-pdf-viewer/troubleshooting |
@Hollow-Ego First of all, let me thank you. You've put a lot of effort into describing and analyzing this issue. I don't see this every day. That's just great! |
@Hollow-Ego Actually, I suggested (or tried to suggest?) using an absolute path. You know your directory layout, so you can easily use absolute paths. I can't, because every developers uses their own directory layout. You explanation why there's a duplicate
Did you see https://pdfviewer.net/extended-pdf-viewer/options? If you did, please tell me, because it means I should improve the page. Every option documented over there can be overwritten. Generally speaking, you've got it right: I tried to make sure that the viewer always uses the correct path automatically. I didn't hear complaints for a long time, and that's why I'm so surprised. Maybe you've hit a rare corner case? |
@stephanrauh You're welcome. And thank your for the quick replies and overall the fact, that you maintain this library. I know my colleagues really love it too. |
@stephanrauh Ah sorry I misunderstood that. I ended up experimenting with an absolute path in the end anyway. I was trying to set the assets folders with an absolute path, until I realized that I was heading in the wrong direction. I will try to set an absolute path and see if this works as expected. After that I will post result here.
Actually I do believe that your showcase has the same problem. I noticed a few teams when switching between the different examples, the viewer sometimes wouldn't load. Unfortunately I never looked in the console and right now when I was trying to naturally get to the same state it was all working fine. However, as described above, I was able to reproduce this error on the showcase page, using the debug tools. right until
I was wondering that too and it is likely, that this is more of an edge. But it also likely that during development everything is fine and only the end user notice the file not loading. And a user might just assume the file hasn't loaded correctly and just tries again. In our case we've also had no problems during development and only noticed this in a staging environment. |
…f the default options
As promised, here is the update and what seems to work for all cases where we need to load the worker files. // In the component where the PDF Viewer is used
// import { DOCUMENT } from '@angular/common';
private document = inject(DOCUMENT);
constructor() {
pdfDefaultOptions.workerSrc = () => {
const baseURI = this.document.baseURI;
const assetsFolder = pdfDefaultOptions.assetsFolder;
const pdfJsVersion = getVersionSuffix(assetsUrl(assetsFolder));
const fileEnding = pdfDefaultOptions.needsES5 ? '-es5.mjs' : '.mjs';
return `${baseURI}${assetsFolder}/pdf.worker-${pdfJsVersion}${fileEnding}`;
};
} I split the declarations up to make it easier to modify the structure of the returned workerSrc URI. This way, if anything changes we don't have to remember to update it for es5 and non-es5 versions. Note though, that we could omit this es5 decision, if we always know exactly if we need it or not. But to have this example work more generally and be close to the current default implementation, I kept it. I could imagine using this implementation as a default. I don't think it would be a breaking change (I might be wrong though) and is a reasonable default. Then with the addition of the improved documentation this option could be overwritten if needed. |
Using the |
Maybe it's also a good idea to check whether the URL of the worker exists. That would allow us to use a different approach to generate the path if the default approach fails, or to detect stuff like the duplicate async function checkUrlExists(url) {
try {
const response = await fetch(url, { method: 'HEAD' });
if (response.ok) {
// TODO: check if the MIME type is correct
console.log('URL exists:', response.status);
return true;
} else {
console.log('URL does not exist:', response.status);
return false;
}
} catch (error) {
console.log('Error:', error);
return false;
}
}
checkUrlExists(pdfDefaultOptions.workerSrc()); And maybe we should load and create the worker in TypeScript. IMHO the current implementation is hard to understand and maintain. Here it is: let { workerSrc } = PDFWorker;
try {
// Wraps workerSrc path into blob URL, if the former does not belong
// to the same origin.
if (
typeof PDFJSDev !== "undefined" &&
PDFJSDev.test("GENERIC") &&
!PDFWorker._isSameOrigin(window.location.href, workerSrc)
) {
workerSrc = PDFWorker._createCDNWrapper(
new URL(workerSrc, window.location).href
);
}
// modified by ngx-extended-pdf-viewer #1512
const worker = new Worker(this.#generateTrustedURL(workerSrc), { type: "module" });
// end of modification by ngx-extended-pdf-viewer #1512 In theory, we could load the worker thread as a blob, but that approach breaks CSP (Content Security Policy). |
Agreed. Having a fallback and a worse case error message is more safe. Though I don't think we would be able to check if the path contains duplicate
I mean it's fine, but if you have to change it often, then it makes sense to improve it |
@stephanrauh Is this bug being fixed and in what version is it expected to be applied? |
@linhhn-familysoft Unfortunately, this ticket got lost in the big pile of new tickets. However, I suspected this ticket is mostly solved. I just wanted to check if using the However, you wouldn't ask if everything worked fine. Can you tell me something about your bug? |
Hi there, hello,
we noticed that sometimes the pdf viewer breaks. We haven't found a consistent way to reproduce the error. Following is what I've learned from my investigations.
This happens in versions 20.5.0 and 21.3.8.
Description
We are using the pdf viewer in a dialog. Opening and closing the dialog multiple times sometimes leads to the pdf viewer breaking. The following error is shown in the console
![grafik](https://private-user-images.githubusercontent.com/47663586/365112220-4776fe27-44e6-47fc-aad1-4a0a0726e4fa.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NTYxMDAsIm5iZiI6MTczODk1NTgwMCwicGF0aCI6Ii80NzY2MzU4Ni8zNjUxMTIyMjAtNDc3NmZlMjctNDRlNi00N2ZjLWFhZDEtNGEwYTA3MjZlNGZhLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDE5MTY0MFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTllZDE0NzdhNjg1Y2M5Y2FiNTI3YmUzZWQ5MzBjY2Q2MDU0OGM1OGQyZGIxYjdiMDJmMmU3OGZlZjgxNDM4OTkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.d-MEmtBZCscFpZZXjJ8b9kx0dpU7tgj3jxMQIeqlAvg)
Transcript:
This error occurs rather rarely and can only be solved for the user by completely refreshing the page. However using the debug tools one can force this error. I believe this means, that the cause is not coming from our application. The viewer is destroyed every time the dialog is closed.
General Steps to Reproduce
As said above, there is no reliable way to naturally reproduce this error. Following the steps below one can force this error to occur through the debug tools.
if (!this.destroyed && t)
in thei.on("test", (t => {
blockreturn shadow(this, "_setupFakeWorkerGlobal", (async () => {
instatic get _setupFakeWorkerGlobal()
t
to ``falseWorkarounds
There is no workaround to my knowledge.
Edit: You could just copy the pdf.worker files to /assets/assets or add a redirect to your server that resolves /assets/assets to just /assets
Possible cause
For this error to naturally occur I suspect that the worker was not cleaned up entirely or correctly. Maybe the browser was too slow to clean it up before the next time opening the viewer. Again, as this is inconsistent and rarely occurs it is hard to say.
The cause for the error message itself lies in the dynamic import for the pdf.worker file.
![grafik](https://private-user-images.githubusercontent.com/47663586/365114946-e157de0a-a297-47d1-b45c-1c917fa307fc.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NTYxMDEsIm5iZiI6MTczODk1NTgwMSwicGF0aCI6Ii80NzY2MzU4Ni8zNjUxMTQ5NDYtZTE1N2RlMGEtYTI5Ny00N2QxLWI0NWMtMWM5MTdmYTMwN2ZjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDE5MTY0MVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTUxMWJiODUyOWRmMGE5ODQ5M2Q5Y2NkYzJiNTgyYzkzYjkxOTYxNDczMjhmZGMyOGRjNjMxNGVkMmQyODA0OGImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.rIBd2cJYG_kVO4uI1efIn6UeTt1c6iMz9uMhLxag8Jc)
(await import(this.workerSrc)
this.workerSrc
is set to './assets/pdf.worker-4.5.732.min.mjs'. However the import function, to my knowledge, tries to resolve this relative to where it is located. Since theviewer
file is already located under "/assets" it tries to resolve it to "assets/assets"Possible solution
From what I've gathered from the source code the workerSrc is `the combination of the assetsFolder and the worker filename.
So a solution could be to either allow the configuration of the workerSrc option, same as with the workerPort or to just drop the usage of the assetsFolder. Given the current documentation, it would probably be preferred to not expose the workerSrc. This could look like this
![grafik](https://private-user-images.githubusercontent.com/47663586/365115935-f808ea58-7905-4f6b-b68d-8f5616b2a309.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NTYxMDEsIm5iZiI6MTczODk1NTgwMSwicGF0aCI6Ii80NzY2MzU4Ni8zNjUxMTU5MzUtZjgwOGVhNTgtNzkwNS00ZjZiLWI2OGQtOGY1NjE2YjJhMzA5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDE5MTY0MVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTkyNDBmZmM4YzM4YjQ0ZGJjMWFjYzJjNGI3MTYyZWRmZDVjNjAzZjFmNjUyMmVhMzBjYjU5YzEwNzA4YjA5MDMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.k3DfraBzCqlf7TgvK2qPSuZkR40fGy-d05XZXOwuofU)
Since I don't know how this could be setup in a test or if this could have any other impacts that I'm not aware of, I haven't made a PR for this fix. But I would be willing to make one
The text was updated successfully, but these errors were encountered: