firebase-ios-sdk icon indicating copy to clipboard operation
firebase-ios-sdk copied to clipboard

Completion closure on putData to upload a file to Firebase Storage is never called

Open filipealva opened this issue 4 years ago • 8 comments

Step 0: Are you in the right place?

  • For issues or feature requests related to the code in this repository file a Github issue.
    • If this is a feature request please use the Feature Request template.
  • For general technical questions, post a question on StackOverflow with the firebase tag.
  • For general (non-iOS) Firebase discussion, use the firebase-talk google group.
  • For backend issues, console issues, and other non-SDK help that does not fall under one of the above categories, reach out to Firebase Support.
  • Once you've read this section and determined that your issue is appropriate for this repository, please delete this section.

[REQUIRED] Step 1: Describe your environment

  • Xcode version: 11.0
  • Firebase SDK version: Latest
  • Firebase Component: Storage
  • Component version: Latest
  • Installation method: CocoaPods

[REQUIRED] Step 2: Describe the problem

When running a putData method in an iPhone without any internet connection it never calls the completion callback.

When monitoring the progress/status of an upload task it never gets called too, i.e. we as developers will get stuck and won't get any response from the Firebase SDK when trying to upload a file without internet connection. We can't just show a connection error for our users because we don't get notified when it does happen, and the "completion" that should be called even when there's an error doesn't get called.

Steps to reproduce:

Just try to upload a file without any internet connection.

Relevant Code:

No need.

filipealva avatar Apr 06 '20 02:04 filipealva

Related stackoverflow discussions:

https://stackoverflow.com/a/61046081 https://stackoverflow.com/a/42536475/556617

paulb777 avatar Apr 06 '20 15:04 paulb777

@filipealva Thanks for filing this.

We don't raise any events if the network is unavailable since this is not an error condition for our SDK. The networking library we use will retry the upload when the device gains network connectivity. We should probably use our progress events to let you know when the upload begins to stall (similar to https://github.com/firebase/firebase-ios-sdk/issues/5298), but this is not something we do right now.

We will prioritize requests like these and let you know when this is part of our roadmap.

schmidt-sebastian avatar Apr 08 '20 17:04 schmidt-sebastian

Guys,

I completely understand that it's not an Firebase SDK error condition, but:

  1. Usually the file upload will come from an user interaction. In the case of a internet connection error, how would we give the user feedback and decide how to proceed in the app without the callback being called. The upload operation has failed. Doesn't matter if it will retry.

  2. Even if the SDK retries. What if the user closed the application? I'll lose track of the progress observer. How I'll grab the downloadUrl for this file?

I'm checking the internet connection before firing the upload operation, but it looks like unnecessary workaround. I mean, there's a "completion" block, the operation failed, it should be called in order for the app to flow, even if there's a retry.

Also, if you use the Firebase SDK with logging enabled you'll see that the network error is logged in the console. I see no reason for not calling the completion block, to be honest.

Thank you guys for making the things clearer for me, I'll keep with the internet connection check workaround and hope to see the completion block being called for a network error in a future version of the SDK

filipealva avatar Apr 08 '20 20:04 filipealva

Besides checking for internet connection I ended up implementing a wrapper + a timer to force the completion call when Firebase Storage fails to run the callback for the put operation within a time interval (in my case 20 seconds).

I'll share the idea here just in case someone needs the same behaviour than me and is wondering an alternative:

        var hasRanCompletion = false
        let operationTimer = Timer(timeInterval: 20, repeats: false) { timer in
            timer.invalidate()
            guard !hasRanCompletion else {
                return
            }

            DeliveryProofManager.addPendingUpload(deliveryProofUpload)
            completion?(nil)
        }

        RunLoop.current.add(operationTimer, forMode: .common)

        let reference = fileStorage.reference().child("your_path")
        let task = reference.putData(deliveryProofUpload.data, metadata: nil) { _, error in
            hasRanCompletion = true
            operationTimer.invalidate()
            guard error == nil else {
                DeliveryProofManager.addPendingUpload(deliveryProofUpload)
                completion?(nil)
                return
            }

            reference.downloadURL { url, error in
                guard let url = url?.absoluteString, error == nil else {
                    DeliveryProofManager.addPendingUpload(deliveryProofUpload)
                    completion?(nil)
                    return
                }

                if isPendingUpload {
                    DeliveryProofManager.updateStop(deliveryProofUpload, withUrl: url)
                    DeliveryProofManager.removePendingUpload(deliveryProofUpload)
                }

                completion?(url)
            }
        }

        task.observe(.success) { _ in
            hasRanCompletion = true
        }

        task.observe(.failure) { _ in
            hasRanCompletion = true
        }

Basically it tries to upload a file and if the Firebase Storage put's method doesn't call the completion callback in 20 seconds it will call it and use another mechanism that I've implemented to queue the pending upload based on internet reachability.

Why I did it:

  • It allows me to present an alternative UI flow for the user when I know that the upload doesn't went through in the first try and we don't now for sure when it will be uploaded;

  • It brings the retry management of the upload for the application's domain, instead of leaving it for the Firebase Storage SDK to upload it. Then you can persist the information regarding the pending upload and retry it in the future without losing track of the task status if the user closes the app for instance.

So if you have similar requirements, something like that is probably going to solve your issue.

filipealva avatar Apr 14 '20 21:04 filipealva

Hi @filipealva This feature has not yet been addressed.

We're in the process of converting the Storage implementation to Swift. See #9963. After that we plan to investigate ways in which the Swift Concurrency Model can help address outstanding feature requests including this one.

Suggestions and PRs appreciated.

paulb777 avatar Jun 30 '22 19:06 paulb777

@paulb777 thanks for the update 🙌

filipealva avatar Jun 30 '22 19:06 filipealva

Hello @paulb777, weirdly enough the storage SDK is at least now throwing the Network connection error with Firebase/Storage > 8.2.0

Here's an example of an error throwed by storage SDK.

NSLocalizedDescription=The Internet connection appears to be offline., NSErrorFailingURLStringKey=https://securetoken.googleapis.com/v1/token?key=, NSErrorFailingURLKey=https://securetoken.googleapis.com/v1/token?key, _kCFStreamErrorDomainKey=1}}, ResponseErrorCode=-13020

So this isn't expected?

thomasdelgado avatar Jun 30 '22 19:06 thomasdelgado

@thomasdelgado I'm not able to reproduce. I see the Network connection error in the console log, but the SDK indefinitely keeps trying to do the upload.

paulb777 avatar Jun 30 '22 22:06 paulb777