Skip to content
This repository has been archived by the owner on Mar 16, 2019. It is now read-only.

Deleting photos from CameraRoll reports as success but they are still there #479

Open
fxfactorial opened this issue Aug 13, 2017 · 14 comments

Comments

@fxfactorial
Copy link

fxfactorial commented Aug 13, 2017

I'm deleting files from the camera roll, they have paths like:

"assets-library://asset/asset.JPG?id=6A0D5C5D-5839-49B5-878A-52AC130B2C9C&ext=JPG"

and I am doing an array of them, so code is like:

const delete_from_cameraroll = delete_these => {
  const d = delete_these.filter(e => e !== null);
  console.log({d});
  return Promise.all(
    d.map(path => {
      return RNFetchBlob.fs
        .unlink(path)
        .then(() => console.log('Success'))
        .catch(err => {
          console.log(err);
        });
    })
  );
};

And the success case goes off but these photos are still in the camera roll.

Versions:

"react-native": "0.45.1",
"react-native-fetch-blob": "^0.10.8",

I was wondering if this could be something related to missing a permission in Info.plist, but when that happens, apps usually crash and in this case the app does not crash.

@lll000111
Copy link
Contributor

lll000111 commented Aug 14, 2017

I'm preparing a PR from my own repo (https://github.com/lll000111/react-native-fetch-blob/tree/0.10.9) that addresses such issues. There were quite a few unused return values, some low-level OS methods don't throw an error when something didn't work but they return false, and often those return values are not looked at, expecting exceptions when things don't work.

Is this Android or iOS?

On Android I may have caught this problem with this new check of the result: https://github.com/lll000111/react-native-fetch-blob/blob/0.10.9/android/src/main/java/com/RNFetchBlob/RNFetchBlobFS.java#L459

@fxfactorial
Copy link
Author

Was for iOS but actually its not so much this libraries responsibility , at least on iOS.

facebook/react-native#15481

@lll000111
Copy link
Contributor

Hmm, according to https://github.com/wkh237/react-native-fetch-blob/blob/master/ios/RNFetchBlob/RNFetchBlob.m#L265 you should at least get an error from RNFB on iOS, since it does an explicit fileExistsAtPath check after removing the file...

@fxfactorial
Copy link
Author

Nope, no errors. Hence painful false positives

@lll000111
Copy link
Contributor

The RN patch you link to won't have an effect though if you use RNFB to remove files AFAICS.

@fxfactorial
Copy link
Author

Hence I said i don't think its this library's responsibility.

@lll000111
Copy link
Contributor

Well, you left the ticket open so I assumed it is... open.

@fxfactorial
Copy link
Author

that came up in discussion with you.

@lll000111
Copy link
Contributor

In any case, you did use RNFB to delete files, and whether or not it should have worked, if not you should at least have received an error and not a success for a deletion that did not happen. So this may still be a RNFB issue even if you switch to an RN method for deletion and won't need a solution any more, I think.

@fxfactorial
Copy link
Author

yes, in any case RNFB needs to throw an error

@lll000111
Copy link
Contributor

Then the issue should be left open? You can unsubscribe if you don't need the followup discussion. I care because I've been working on the error reporting of this package just those past days...

@wkh237
Copy link
Owner

wkh237 commented Aug 16, 2017

Okay, I think this is incorrect, will see if it's possible to handle it. Otherwise, it should throw an error.

@lll000111
Copy link
Contributor

lll000111 commented Sep 3, 2017

As a task item / TODO:

https://github.com/wkh237/react-native-fetch-blob/blob/master/ios/RNFetchBlob/RNFetchBlob.m#L265

I think unlink for iOS needs to check the boolean return value of removeItemAtPath. It can be NO without an error, in which case our function doesn't report a problem.

There also is this strange report about something that looks like an iOS issue: https://stackoverflow.com/a/37055832/544779

Also: https://stackoverflow.com/a/5683277/544779
We just delete, various examples on SO all at least call isDeletableFileAtPath before trying to delete the file.

@lll000111 lll000111 added the task label Sep 3, 2017
@lll000111
Copy link
Contributor

lll000111 commented Sep 6, 2017

I added a small patch to the unlink function on iOS in my own 0.10.9 branch, if anyone wants to test if that solves this problem (I can't): https://github.com/lll000111/react-native-fetch-blob/tree/0.10.9

The patch: https://github.com/lll000111/react-native-fetch-blob/commit/97e14ed689516c1934e526cd163cbe8e3fe04b4c

@lll000111 lll000111 removed the task label Sep 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants