wonday / react-native-pdf

A <Pdf /> component for react-native
MIT License
1.6k stars 557 forks source link

App Crash with this.lastRNBFTask.cancel is not a function #796

Open riteshshah5432 opened 11 months ago

riteshshah5432 commented 11 months ago

react-native version: 0.72.4

react-native-pdf version: 6.7.4

What platform does your issue occur on? : Both

Describe your issue as precisely as possible : 1) When the PDF is loading and try to back to the screen that time app crashes 2) logs TypeError: this.lastRNBFTask.cancel is not a function (it is undefined)

Screenshot Simulator Screenshot - iPhone 15 Pro - 2023-12-23 at 18 09 22

Here is the code: <Pdf trustAllCerts={false} source={{ uri: URL }} onError={(error) => { console.log(error); }} onLoadComplete={(numberOfPages, filePath) => { console.log('completed') }} renderActivityIndicator={() => ()} style={style.pdf} />

Subiyel commented 11 months ago

same issue

mdalishanali commented 10 months ago

do you find anything?

minh-dai commented 10 months ago

+1

kartavyaparekh96 commented 10 months ago

+1

alex-strae commented 10 months ago

I have the same issue when browsing fast between different unloaded PDFs and download does not have time to finish before I start fetching new PDF.

This is due to this code introduced and merged about 2 weeks ago. In index.js, line 279, with declaration of this.lastRNBFTask.

BAD CODE: .catch(async (error) => { this._onError(error); });

comment it out and problem solved.

I am not an very experienced dev, but my guess is that the catch intercepts the cancel call that is supposed to be sent to ReactNativeBlobUtil and run its course there as it should, so instead it doesn't, and the code asks where the hell is my cancel method? Crash.

Well we caught the error before the cancel could do it's thing...

There is another similar catch on line 323. That one is fine, let it be.

mdalishanali commented 10 months ago

so, what should be better approach?

On Fri, Jan 5, 2024 at 3:04 PM alex-strae @.***> wrote:

I have the same issue when browsing fast between different unloaded PDFs and download does not have time to finish before I start fetching new PDF.

This is due to this code introduced and merged about 2 weeks ago. In index.js, line 279, with declaration of this.lastRNBFTask.

BAD CODE: .catch(async (error) => { this._onError(error); });

comment it out and problem solved.

I am not an very experienced dev, but my guess is that the catch intercepts the cancel call that is supposed to be sent to ReactNativeBlobUtil and run its course there as it should, so instead it doesn't, and the code asks where the hell is my cancel method? Crash.

Well we catched the error before the cancel could do it's thing...

There is another similar catch on line 323. That one is fine, let it be.

— Reply to this email directly, view it on GitHub https://github.com/wonday/react-native-pdf/issues/796#issuecomment-1878376691, or unsubscribe https://github.com/notifications/unsubscribe-auth/AV7OUDU2GGWOLRCCXOFOW3TYM7CJFAVCNFSM6AAAAABBAYXAFKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZYGM3TMNRZGE . You are receiving this because you commented.Message ID: @.***>

alex-strae commented 10 months ago

Well, what do you want to do? For me, I had to have the app stop crashing when browsing fast between PDFs. The better approach for me was to comment out the recently introduced code (that I specified in last post).

mdalishanali commented 10 months ago

cool, thanks, I will also do like that.

On Fri, Jan 5, 2024 at 3:25 PM alex-strae @.***> wrote:

Well, what do you want to do? For me, I had to have the app stop crashing when browsing fast between PDFs. The better approach for me was to comment out the recently introduced code (that I specified in last post).

— Reply to this email directly, view it on GitHub https://github.com/wonday/react-native-pdf/issues/796#issuecomment-1878403772, or unsubscribe https://github.com/notifications/unsubscribe-auth/AV7OUDQRPAZFOZY6UHTLLCLYM7EZ3AVCNFSM6AAAAABBAYXAFKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZYGQYDGNZXGI . You are receiving this because you commented.Message ID: @.***>

kartavyaparekh96 commented 10 months ago

I have downgrade version @6.7.2 its working fine

alex-strae commented 10 months ago

Yes, because the code causing the issue was introduced just two weeks ago.HälsningarAlex5 jan. 2024 kl. 11:26 skrev kartavyaparekh96 @.***>: I have downgrade version @6.7.2 its working fine

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

AminDannak commented 10 months ago

I have downgrade version @6.7.2 its working fine

for me, it lead to a build issue. i used 6.7.3 and it seems fine. no crash

shahid-0 commented 10 months ago

I have the same issue when browsing fast between different unloaded PDFs and download does not have time to finish before I start fetching new PDF.

This is due to this code introduced and merged about 2 weeks ago. In index.js, line 279, with declaration of this.lastRNBFTask.

BAD CODE: .catch(async (error) => { this._onError(error); });

comment it out and problem solved.

I am not an very experienced dev, but my guess is that the catch intercepts the cancel call that is supposed to be sent to ReactNativeBlobUtil and run its course there as it should, so instead it doesn't, and the code asks where the hell is my cancel method? Crash.

Well we caught the error before the cancel could do it's thing...

There is another similar catch on line 323. That one is fine, let it be.

I also have the same issue, when I press back button it arise the same error, then I saw your comment and try to do the same thing as you mentioned. But still it arise error. It didn't crash the app but arise error on the onError props, but I don't want to arise any error on the cancel. Then I commented out the below code on the line 146, and it works for me.

// componentWillUnmount() {
//     this._mounted = false;
//     if (this.lastRNBFTask) {
//         this.lastRNBFTask.cancel(err => {
//         });
//         this.lastRNBFTask = null;
//     }
// }
abanobmikaeel commented 10 months ago

Also happening to me any way to solve? Happening on latest also. @shahid-0 how does one deploy with commented out code? Is that possible?

alex-strae commented 10 months ago

Make change(s) in node_modules, cd /ios , run pod install and you’re good.HälsningarAlex23 jan. 2024 kl. 14:34 skrev Abanob Mikaeel @.***>: Also happening to me any way to solve? Happening on latest also

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

UgurGumushan commented 9 months ago

+1 This is a blocker and I can't update this package anymore

IrfanVN commented 9 months ago

I have the same issue when browsing fast between different unloaded PDFs and download does not have time to finish before I start fetching new PDF.

This is due to this code introduced and merged about 2 weeks ago. In index.js, line 279, with declaration of this.lastRNBFTask.

BAD CODE: .catch(async (error) => { this._onError(error); });

comment it out and problem solved.

I am not an very experienced dev, but my guess is that the catch intercepts the cancel call that is supposed to be sent to ReactNativeBlobUtil and run its course there as it should, so instead it doesn't, and the code asks where the hell is my cancel method? Crash.

Well we caught the error before the cancel could do it's thing...

There is another similar catch on line 323. That one is fine, let it be.

Wow this is working really well. Spend alot of time on this issue and thank god u have solved my issue

Kashifmub205 commented 9 months ago

I was having the same issue. reinstalling the package with the latest one solved my crashing issue but then it was not crashing rather it was giving reactnativeblobutil error. by adding trustAllCerts={false} solved the not displaying pdf issue.

  <PDFView
          style={styles.pdfContainer}
          source={{uri: link}}
          onError={onError}
          trustAllCerts={false}
        />
KarlosQ commented 9 months ago

I created this patch (patch-package) for avoid this crash:

react-native-pdf+6.7.4.patch

diff --git a/node_modules/react-native-pdf/index.js b/node_modules/react-native-pdf/index.js
index c7c58d8..70a6057 100644
--- a/node_modules/react-native-pdf/index.js
+++ b/node_modules/react-native-pdf/index.js
@@ -127,7 +127,7 @@ export default class Pdf extends Component {

         if ((nextSource.uri !== curSource.uri)) {
             // if has download task, then cancel it.
-            if (this.lastRNBFTask) {
+            if (this.lastRNBFTask?.cancel) {
                 this.lastRNBFTask.cancel(err => {
                     this._loadFromSource(this.props.source);
                 });
@@ -145,7 +145,7 @@ export default class Pdf extends Component {

     componentWillUnmount() {
         this._mounted = false;
-        if (this.lastRNBFTask) {
+        if (this.lastRNBFTask?.cancel) {
             this.lastRNBFTask.cancel(err => {
             });
             this.lastRNBFTask = null;
@@ -250,7 +250,7 @@ export default class Pdf extends Component {

     _downloadFile = async (source, cacheFile) => {

-        if (this.lastRNBFTask) {
+        if (this.lastRNBFTask?.cancel) {
             this.lastRNBFTask.cancel(err => {
             });
             this.lastRNBFTask = null;
neerajsingh-dev commented 9 months ago

For me only @KarlosQ patch is working

elkinjosetm commented 8 months ago

Even-though the patch prevents the app from crashing, it's not actually fixing the problem, because the cancel is not triggered so the download keeps going in background, which is not ideal.

Downgrading to 6.7.3 seems a better workaround imo, as it actually cancel the download as expected.

KarlosQ commented 8 months ago

@wonday Do you see a solution for this 6.7.4 regression?

UgurGumushan commented 8 months ago

I created this patch (patch-package) for avoid this crash:

react-native-pdf+6.7.4.patch

diff --git a/node_modules/react-native-pdf/index.js b/node_modules/react-native-pdf/index.js
index c7c58d8..70a6057 100644
--- a/node_modules/react-native-pdf/index.js
+++ b/node_modules/react-native-pdf/index.js
@@ -127,7 +127,7 @@ export default class Pdf extends Component {

         if ((nextSource.uri !== curSource.uri)) {
             // if has download task, then cancel it.
-            if (this.lastRNBFTask) {
+            if (this.lastRNBFTask?.cancel) {
                 this.lastRNBFTask.cancel(err => {
                     this._loadFromSource(this.props.source);
                 });
@@ -145,7 +145,7 @@ export default class Pdf extends Component {

     componentWillUnmount() {
         this._mounted = false;
-        if (this.lastRNBFTask) {
+        if (this.lastRNBFTask?.cancel) {
             this.lastRNBFTask.cancel(err => {
             });
             this.lastRNBFTask = null;
@@ -250,7 +250,7 @@ export default class Pdf extends Component {

     _downloadFile = async (source, cacheFile) => {

-        if (this.lastRNBFTask) {
+        if (this.lastRNBFTask?.cancel) {
             this.lastRNBFTask.cancel(err => {
             });
             this.lastRNBFTask = null;

Would you like to create a pull request with those changes?

kamiltenkai commented 8 months ago

Hey. Adding "cache: true" to source helped me. Like this: <Pdf source={{ uri: url, cache: true }}

nawazokz commented 8 months ago

I have the same issue when browsing fast between different unloaded PDFs and download does not have time to finish before I start fetching new PDF. This is due to this code introduced and merged about 2 weeks ago. In index.js, line 279, with declaration of this.lastRNBFTask. BAD CODE: .catch(async (error) => { this._onError(error); }); comment it out and problem solved. I am not an very experienced dev, but my guess is that the catch intercepts the cancel call that is supposed to be sent to ReactNativeBlobUtil and run its course there as it should, so instead it doesn't, and the code asks where the hell is my cancel method? Crash. Well we caught the error before the cancel could do it's thing... There is another similar catch on line 323. That one is fine, let it be.

I also have the same issue, when I press back button it arise the same error, then I saw your comment and try to do the same thing as you mentioned. But still it arise error. It didn't crash the app but arise error on the onError props, but I don't want to arise any error on the cancel. Then I commented out the below code on the line 146, and it works for me.

// componentWillUnmount() {
//     this._mounted = false;
//     if (this.lastRNBFTask) {
//         this.lastRNBFTask.cancel(err => {
//         });
//         this.lastRNBFTask = null;
//     }
// }

thanks bro, that's work for me

dariyd commented 8 months ago

I created this patch (patch-package) for avoid this crash:

react-native-pdf+6.7.4.patch

diff --git a/node_modules/react-native-pdf/index.js b/node_modules/react-native-pdf/index.js
index c7c58d8..70a6057 100644
--- a/node_modules/react-native-pdf/index.js
+++ b/node_modules/react-native-pdf/index.js
@@ -127,7 +127,7 @@ export default class Pdf extends Component {

         if ((nextSource.uri !== curSource.uri)) {
             // if has download task, then cancel it.
-            if (this.lastRNBFTask) {
+            if (this.lastRNBFTask?.cancel) {
                 this.lastRNBFTask.cancel(err => {
                     this._loadFromSource(this.props.source);
                 });
@@ -145,7 +145,7 @@ export default class Pdf extends Component {

     componentWillUnmount() {
         this._mounted = false;
-        if (this.lastRNBFTask) {
+        if (this.lastRNBFTask?.cancel) {
             this.lastRNBFTask.cancel(err => {
             });
             this.lastRNBFTask = null;
@@ -250,7 +250,7 @@ export default class Pdf extends Component {

     _downloadFile = async (source, cacheFile) => {

-        if (this.lastRNBFTask) {
+        if (this.lastRNBFTask?.cancel) {
             this.lastRNBFTask.cancel(err => {
             });
             this.lastRNBFTask = null;

This worked for me. Many thanks!

wonday commented 8 months ago

Would you like to create a pull request with those changes?

abanobmikaeel commented 7 months ago

Isn't that code kinda overwriting needed functionality? @wonday

As @elkinjosetm mentioned:

Even-though the patch prevents the app from crashing, it's not actually fixing the problem, because the cancel is not triggered so the download keeps going in background, which is not ideal.

chandwho commented 7 months ago

I was facing the same issue. Updating to the latest version 6.7.5 has fixed it.

alex-strae commented 6 months ago

I updated to 6.7.5 recently (RN 0.74.1, Android: Fabric Disabled, Bridgeless Disabled, TurboModules enabled | iOS new architecture disabled) and the problem was back. Crash when quickly changing source for PdfViewer (this.lastRNBFTask.cancel is not a function) by scrolling between different "dates" with different pdf sources.

I applied Karlos patch. While it does work, if I scroll back to a date I just skipped, it's noticeable that there was no cancel fired. The PDF was loaded in the background even though I switched to another date.

The reason for this should be that the error manifested itself if this.lastRNBFTask.cancel was undefined, as in it was falsy. This means the patch will make the function never run if it doesn't exist. While that is good, wouldn't it be better if the function existed for us and we could run it? Yes.

I tried my old patch I posted a while ago which is simple: comment out catch-code on line 279, after the creation of the lastRNBFTask object creation.

/ .catch(async (error) => { this._onError(error); }); /

And now everything works as intended (I think?). The reasons it works, I believe, is that when we reset the pdf source fast, there is an error in the creation of the lastRNBFTask object. This error get's caught in the catch and the lastRNBFTask object gets corrupted, or at least interrupted in its creation. There is no cancel method available, and that causes the crash (.cancel is undefined). By disabling the catch, the lastRNBFTask object can get created with all it's helper methods (such as the .cancel) even though another process before it such as the .progress or .fetch had an error.

iduke-rbi commented 6 months ago

The above suggestion from @alex-strae worked for me with Expo 50 and 6.7.5.

I was getting a crash with stacktrace TypeError: undefined is not a function at componentDidUpdate. This no longer crashes with the catch block commented out.

roniemartinez commented 5 months ago

This happens to us when pressing back button on Stack (expo-router) while PDF is not completely loaded. While not a proper solution, we have made a workaround by preventing back until PDF is loaded (https://reactnavigation.org/docs/preventing-going-back/). Looking forward to the proper fix.

tsalama commented 5 months ago

This happens to us when pressing back button on Stack (expo-router) while PDF is not completely loaded. While not a proper solution, we have made a workaround by preventing back until PDF is loaded (https://reactnavigation.org/docs/preventing-going-back/). Looking forward to the proper fix.

Same issue for us!

jaiieth commented 5 months ago

I believe the issue occurs because this.lastRNBFTask is unintentionally assigned a Promise object on line 262:

this.lastRNBFTask = ReactNativeBlobUtil.config({
    // response data will be saved to this path if it has access rights.
    path: tempCacheFile,
    trusty: this.props.trustAllCerts,
})
    .fetch(
        source.method ? source.method : 'GET',
        source.uri,
        source.headers ? source.headers : {},
        source.body ? source.body : ""
    )
    // listen to the download progress event
    .progress((received, total) => {
        if (this.props.onLoadProgress) {
            this.props.onLoadProgress(received / total);
        }
        if (this._mounted) {
            this.setState({ progress: received / total });
        }
    })
    .catch(async (error) => { // <- remove this
        this._onError(error);
    });

The issue arises because ReactNativeBlobUtil.fetch() returns an extended Promise object, which includes .cancel() method. However, calling .catch() on this extended Promise returns a normal Promise, which does not have the .cancel() method. This results in an error when attempting to call this.lastRNBFTask.cancel().

Removing the .catch() as suggested by @alex-strae should resolve the crash.

Additionally, when calling .cancel(), ReactNativeBlobUtil will reject the Promise, triggering the onError method.

To handle this properly, modify line 321 as follows:

.catch(async (error) => {
    this._unlinkFile(tempCacheFile);
    this._unlinkFile(cacheFile);
    if (error.name !== 'ReactNativeBlobUtilCanceledFetch') {
        this._onError(error);
    }
});

This change ensures that the onError method is only called for errors other than ReactNativeBlobUtilCanceledFetch.

Maniae commented 1 month ago

https://github.com/wonday/react-native-pdf/issues/796#issuecomment-2180325138 from @jaiieth is the solution that makes the most sense, and actually solves the issue 👍

This is quite strange https://github.com/wonday/react-native-pdf/pull/827 got merged as a "fix" as it only handles unMount crash situations, creates dead code in comments, and does not in fact fixes the issues of changing the pdf source url 😕.

react-native-pdf+6.7.5.patch

diff --git a/node_modules/react-native-pdf/index.js b/node_modules/react-native-pdf/index.js
index b81a141..2b65ee0 100644
--- a/node_modules/react-native-pdf/index.js
+++ b/node_modules/react-native-pdf/index.js
@@ -276,9 +276,6 @@ export default class Pdf extends Component {
                 if (this._mounted) {
                     this.setState({progress: received / total});
                 }
-            })
-            .catch(async (error) => {
-                this._onError(error);
             });

         this.lastRNBFTask
@@ -323,7 +320,9 @@ export default class Pdf extends Component {
             .catch(async (error) => {
                 this._unlinkFile(tempCacheFile);
                 this._unlinkFile(cacheFile);
-                this._onError(error);
+                if (error.name !== 'ReactNativeBlobUtilCanceledFetch') {
+                    this._onError(error);
+                }            
             });

     };