338 - Fix render method missing catch statement on render failure

[dvago]

This solves the issue #338

There are cases when the document loads but pdfPage.render method triggers an error as pdfPage is undefined, not having the catch statement triggers a console error.

@FranckFreiburger given you are not maintaining this package since 5-4 months I'm wondering if you can consider requesting some help and include more maintainers.

[justincpollard]

@dvago are you sure this fixes #338 ? The error message posted there is TypeError: Cannot read properties of undefined (reading 'catch') which seems to indicate the code is trying to read the catch method from something undefined. I've seen this is stacktraces for this lib too, and those seem to point to this block:

pdfRender.cancel().catch(function(err) {
	emitEvent('error', err);
});

Could it be that cancel is undefined on pdfRender? I'm not deeply knowledgable in javascript, but I don't see that the cancel method exists on Promise, which is what I think pdfRender is.

[dvago]

@justincpollard it basically fails within the render method, throwing an undefined so the chain after the render failure breaks. What the snippet does is to complete the promise with nothing but a catch statement to prevent further failures within the thread.

Given the experience for the user is not breaking, the snippet, de facto, silences a random render failure, which eventually gets recalled.

What you are referring to is a method on pdfRender which might exists, you need to inspect the prototype of pdfRender, which is a Promise but has his own custom methods, so using .cancel is totally sensical.

In general, it is best practice to attach the catch statement in any promise, so, in case, for any obscure reason, your promise fails something is ready to catch it.

By the way, you can test the pull request yourself, just place the code change within the file in your project's node_modules/ and try to re-run to see if fixes the error.

[justincpollard]

@dvago forgive me if I'm missing something. I'm only pointing out that issue #338 that you mention in the description seems to be different than what you've solved. This PR is a welcome addition despite that!

[dvago]

Right, so: #338 complains about a type error coming out from the PDFJSWrapper.renderPage which under the hood is calling pdfPage.render, this method fails because of the above and therefore renderPage returns exactly the same breaking issue.

Another possible fix would probably be to check for undefined on line 202 like so:

if (!pdfPage) {
  return;
}

in fact, I would probably add it as well, there is no point to move onward if the pdfPage isn't defined.

[sivanbil]

Excuse me, when will this branch be merged?

[dvago]

I believe all of us are waiting for it :)

[diantongren]

I tried this, but did not work. Please check out the PR#350

[andyowli]

I tried this, but did not work.TypeError: Cannot read properties of undefined (reading 'catch') use version 4.3.0，the wrong direction is pdfRender.cancel().catch

[luisrossi]

• Cannot read properties of undefined (reading 'catch') #377