Skip to content

App crashes when storage().putFile is called with assets-library uri on iOS #1232

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

Closed
smallex opened this issue Jun 20, 2018 · 23 comments
Closed
Assignees
Labels
platform: ios plugin: storage Firebase Cloud Storage type: bug New bug report

Comments

@smallex
Copy link

smallex commented Jun 20, 2018

Issue

I am trying to upload an image from the camera roll of the phone to Firebase Storage using react-native-firebase. However, when I pass the assets-library uri of the image to the putFile method, the app crashes without an error.

For example:

const uri = 'assets-library://asset/asset.JPG?id=3DEE5FA3-9E58-479B-9AD9-A7FDBEDF0502&ext=JPG'; firebase.storage().ref('test.jpeg').putFile(uri) .then(...) .catch(...);

Is this the expected behavior? If yes, I apologize for taking up your time. However, how can I transform the assets uri to a full file path that is recognized by RNFirebase?

Note: This is a repost of my question os Stack Overflow: https://stackoverflow.com/questions/50828924/app-crashes-when-storage-putfile-is-called-with-assets-library-uri-on-ios

Environment

  1. Application Target Platform:
    iOS, iPhone X simulator

  2. Development Operating System:
    macOS High Sierra 10.13.5

  3. Build Tools:
    Xcode: 9.4.1

  4. React Native version:
    0.55.4

  5. RNFirebase Version:
    https://github.com/invertase/react-native-firebase.git#build-issues

  6. Firebase Module:
    storage

@SamMatthewsIsACommonName
Copy link
Contributor

I'm also seeing this. Going to start looking deeper now

@Ehesp
Copy link
Member

Ehesp commented Jun 21, 2018

Could you check the logs within XCode, it should tell you exactly what happened to cause the crash?

@SamMatthewsIsACommonName
Copy link
Contributor

screen shot 2018-06-21 at 4 27 04 pm

Ok which bit should I be looking at? Sorry bit clueless with native stuff

@SamMatthewsIsACommonName
Copy link
Contributor

SamMatthewsIsACommonName commented Jun 21, 2018

Also can confirm same code works on android, except for the standard fix of removing the extra bit of android path file:

    let imageString;
    if (Platform.OS === 'android') {
      imageString = image.replace('file://', '');
    } else {
      imageString = image;
    }

@Salakar Salakar added platform: ios plugin: storage Firebase Cloud Storage labels Jun 21, 2018
@SamMatthewsIsACommonName
Copy link
Contributor

SamMatthewsIsACommonName commented Jun 22, 2018

The uri coming from React Native CameraRoll component (part of RN lib) looks like this:

assets-library://asset/asset.JPG?id=DA485E0A-AA02-4850-8B96-A46B3B1EEF27&ext=JPG

And coming from react native camera kit (works) like this:

file:///private/var/mobile/Containers/Data/Application/E63D710D-1663-4EC5-B048-F8E6178FCCD0/tmp/D16252FC-A299-4891-BFEE-15D1FE4475A4-3054-000001981131C745.jpg

I'm going to try and keep investigating. Maybe that it is in the form of a query string not an absolute path?

@SamMatthewsIsACommonName
Copy link
Contributor

SamMatthewsIsACommonName commented Jun 22, 2018

Ok so I tried a couple of things. First I tried both the promise style putfile and the putfile with progress and both crashed when trying to pass in the camer roll uri. I then tried this based off of @Salakar stack overflow post:

  const image = this.state.photos[index].node.image;
    const uri = image.uri;
    const fileName = image.filename;
    const imagePath = `${firebase.storage.Native.DOCUMENT_DIRECTORY_PATH}/${fileName}`;

Which gives me an image path like this:

/var/mobile/Containers/Data/Application/3224F584-473F-41D9-BAC3-58ACF2FE1459/Documents/IMG_1876.JPG

which doesn't crash it, but then it doesn't upload properly. It receives the progress callback, but then gets an 'unhandled error' in both the listener and promise style ways of structuring it (use ing either the err callback or the .catch)

Edit, is it an issue that the application id is different in those two examples ^ between the one coming from the camera kit photo and the firebase native file path?

@skbellevue
Copy link

skbellevue commented Jun 22, 2018

Yes, app does not crash, but just hangs.
Problem is the that the upload operation is probably done in the main thread, so the dispatch_sync is overkill.

I solved it by doing (in RNFirebaseStorage.m):

if ([NSThread isMainThread]) {
        uploadTask = [fileRef putFile:url metadata:firmetadata];
    } else {
        dispatch_sync(dispatch_get_main_queue(), ^{
            uploadTask = [fileRef putFile:url metadata:firmetadata];
        });
    }

Hope it helps...

@SamMatthewsIsACommonName
Copy link
Contributor

@skbellevue I'll also try and confirm if it works thanks a lot. One question though is this in response to using the uri (which causes the hard crash) or the version where we append the filename to the native document patha nd then it never completes the upload properly and gets the uncaught error?
Thanks

@chrisbianca
Copy link
Contributor

chrisbianca commented Jun 22, 2018

Are you using a detached expo project?
A fix similar to the one mentioned by @skbellevue has been merged in to master pending release to address a threading issue with detached expo projects.

@skbellevue
Copy link

@SamMatthewsIsACommonName - Not sure I follow your question but this is in response of the screenshot you posted. I had the exact same issue while trying to upload a photo from the camera roll.

@SamMatthewsIsACommonName
Copy link
Contributor

Aah @skbellevue I see thanks. Dont worry I was being confusing! Can I ask what line this goes? I'm super unfamiliar with obj-c and dont want to break anything!
@chrisbianca no this is just a vanilla create react native app project

@SamMatthewsIsACommonName
Copy link
Contributor

Sorry ignore htat I found it! was looking in putfile vs uploadFIle...

@SamMatthewsIsACommonName
Copy link
Contributor

Ok working thanks! I'll make sure to keep patching it.
One interesting thing to note is that when choosing putFile from the CameraRoll component image uri, it was using the uploadData method versus uploadFile. Not sure if that makes any difference?

So I put the conditional in both places basically.
Thanks!

@joelgetaction
Copy link

Apple changed the image format to HEIC in iOS 11. Not sure if that's related. Looks like your urls were JPGs but some of the other pickers out there (including Photos) translate HEIC to JPG whereas if you get an asset directly from the library it can be an HEIC and you need to use ALAssetLibrary to read it.

@Salakar
Copy link
Contributor

Salakar commented Jun 25, 2018

@skbellevue thanks for the proposed solution, React has a util to do this, could you try importing RCTUtils and changing the dispatch as per the below (instead of the if statement in your example) - this will check if already on main thread. @skbellevue @SamMatthewsIsACommonName if either of you could try this that'd be great.

Add the import:

#import <React/RCTUtils.h>

Diff

- dispatch_sync(dispatch_get_main_queue(), ^{
+ RCTUnsafeExecuteOnMainQueueSync(^{

Upload task block after changes (no [NSThread isMainThread] check required):

RCTUnsafeExecuteOnMainQueueSync(^{
 	uploadTask = [fileRef putFile:url metadata:firmetadata];
});

@Salakar Salakar self-assigned this Jun 25, 2018
@Salakar Salakar added this to the v4.3.0 Release milestone Jun 25, 2018
@Salakar Salakar added the type: bug New bug report label Jun 25, 2018
@SamMatthewsIsACommonName
Copy link
Contributor

@Salakar will try today and report back ;) thanks

@SamMatthewsIsACommonName
Copy link
Contributor

@Salakar worked!

@Salakar
Copy link
Contributor

Salakar commented Jun 26, 2018

@SamMatthewsIsACommonName nice 👌 thanks for testing, could you submit a patch/pr with the change if you could, can get it merged in for a future release. If not then I can pick this up in a few days.

@SamMatthewsIsACommonName
Copy link
Contributor

@Salakar well as crazy as this may seem because I am a 'one man band' I'm still amazingly unaware of how to do a pr.... I know its insane hahaha I need to learn one day

@Salakar
Copy link
Contributor

Salakar commented Jun 26, 2018

@SamMatthewsIsACommonName no problem, thought I'd ask anyway =] leave it with me

@Salakar Salakar assigned Salakar and unassigned Salakar Jul 1, 2018
@Salakar
Copy link
Contributor

Salakar commented Jul 1, 2018

Thanks both for reporting this issue and helping me debug the issue with a proposed fix.

I will close this issue for now and track it as well as other issues collectively over on the Storage improvements proposal to be addressed in a future release. See #1260

@Salakar Salakar closed this as completed Jul 1, 2018
@Salakar Salakar removed this from the v4.3.0 Release milestone Jul 1, 2018
@Salakar
Copy link
Contributor

Salakar commented Jul 6, 2018

Fixed in #1195 - landing as part of v4.3.0

@Salakar
Copy link
Contributor

Salakar commented Jul 10, 2018

Fix is now live in the v4.3.0 release.


Loving react-native-firebase and the support we provide? Please consider supporting us with any of the below:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: ios plugin: storage Firebase Cloud Storage type: bug New bug report
Projects
None yet
Development

No branches or pull requests

7 participants