FirebaseStorage.StorageError provides limited information

[jurgowski]

Description

One of my TestFlight users consistently gets an error when using the FirebaseStorage API: StorageReference.writeAsync()

Logging this error provides little insights since the localizedDescription is The operation couldn’t be completed. (FirebaseStorage.StorageError error 0.)

Unpacking the error case and logging the .unknown(String) produces An unknown error occurred, please check the server response.

The server response isn't on the error however since its lost when the Objective-C NSError is converted the Swift enum via swiftConvert(objcError:).

Would it be possible to instead make StorageError conform to CustomNSError and expose the userInfo in Swift? Would love to debug this without rewriting a significant portion of my code in Objective-C.

Reproducing the issue

Trying to figure this out myself.

Firebase SDK Version

10.5.0

Xcode Version

14.2

Installation Method

Swift Package Manager

Firebase Product(s)

Storage

Targeted Platforms

iOS

Relevant Log Output

No response

If using Swift Package Manager, the project's Package.resolved

{
  "pins" : [
    {
      "identity" : "abseil-cpp-swiftpm",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/abseil-cpp-SwiftPM.git",
      "state" : {
        "revision" : "583de9bd60f66b40e78d08599cc92036c2e7e4e1",
        "version" : "0.20220203.2"
      }
    },
    {
      "identity" : "boringssl-swiftpm",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/boringssl-SwiftPM.git",
      "state" : {
        "revision" : "dd3eda2b05a3f459fc3073695ad1b28659066eab",
        "version" : "0.9.1"
      }
    },
    {
      "identity" : "firebase-ios-sdk",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/firebase-ios-sdk",
      "state" : {
        "revision" : "f567ed9a2b30e29159df258049a9c662c517688e",
        "version" : "10.5.0"
      }
    },
    {
      "identity" : "googleappmeasurement",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/GoogleAppMeasurement.git",
      "state" : {
        "revision" : "9a09ece724128e8d1e14c5133b87c0e236844ac0",
        "version" : "10.4.0"
      }
    },
    {
      "identity" : "googledatatransport",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/GoogleDataTransport.git",
      "state" : {
        "revision" : "f6b558e3f801f2cac336b04f615ce111fa9ddaa0",
        "version" : "9.2.1"
      }
    },
    {
      "identity" : "googleutilities",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/GoogleUtilities.git",
      "state" : {
        "revision" : "0543562f85620b5b7c510c6bcbef75b562a5127b",
        "version" : "7.11.0"
      }
    },
    {
      "identity" : "grpc-ios",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/grpc/grpc-ios.git",
      "state" : {
        "revision" : "8440b914756e0d26d4f4d054a1c1581daedfc5b6",
        "version" : "1.44.3-grpc"
      }
    },
    {
      "identity" : "gtm-session-fetcher",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/gtm-session-fetcher.git",
      "state" : {
        "revision" : "5ccda3981422a84186387dbb763ba739178b529c",
        "version" : "2.3.0"
      }
    },
    {
      "identity" : "leveldb",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/leveldb.git",
      "state" : {
        "revision" : "0706abcc6b0bd9cedfbb015ba840e4a780b5159b",
        "version" : "1.22.2"
      }
    },
    {
      "identity" : "mixpanel-swift",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/mixpanel/mixpanel-swift",
      "state" : {
        "branch" : "master",
        "revision" : "dfc52c9155f2e42fb69f98a2370647d28d7eebe1"
      }
    },
    {
      "identity" : "nanopb",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/nanopb.git",
      "state" : {
        "revision" : "819d0a2173aff699fb8c364b6fb906f7cdb1a692",
        "version" : "2.30909.0"
      }
    },
    {
      "identity" : "promises",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/promises.git",
      "state" : {
        "revision" : "ec957ccddbcc710ccc64c9dcbd4c7006fcf8b73a",
        "version" : "2.2.0"
      }
    },
    {
      "identity" : "swift-protobuf",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/apple/swift-protobuf.git",
      "state" : {
        "revision" : "0af9125c4eae12a4973fb66574c53a54962a9e1e",
        "version" : "1.21.0"
      }
    }
  ],
  "version" : 2
}

If using CocoaPods, the project's Podfile.lock

Expand Podfile.lock snippet

```yml Replace this line with the contents of your Podfile.lock! ```

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[paulb777]

Thanks for the report. We'll take a look in the next few weeks. In the meantime, let us know if you find anything more - and we'd be interested in a PR.

[paulb777]

Errors from the server should actually be wrapped by func error(withServerError serverError: NSError, ref: StorageReference). if the error is not a server error, all the context should be in the Swift error. More examples and context in #10913. If you're still blocked, we may need a reproducible example.

[jurgowski]

Sorry for the delayed response and thanks for adding a PR!

So the Error that is created via error(withServerError:.. is actually great - it has all the info needed since it copies over the userInfo dictionary from the server Error. #10913 might actually not be necessary as all the info is carried over.

The loss in information comes when the wrapped Error is converted to the new StorageError in the getResultCallback method. I think a fix here would be to just not call .swiftConvert(objcError:. The case where there is no error in the callback can potentially leverage the new StorageError error type introduced.

I was actually able to work around this quite easily (staying in an async Swift context) by just using the sync APIs and wrapping them with withCheckedThrowingContinuation myself. This threw the wrapped error(withServerError error as opposed to the StorageError and had all the info needed in my logging to debug.

TL'DR Server Error -> Wrapped Error -> StorageError. The first two are great - the third is missing info.

[paulb777]

Thanks @jurgowski. I like the ideas of using CustomNSError and/or dropping swiftConvert. However, I'm not sure that can be done without breaking the ability to catch individual StorageErrors so it would be a breaking change. As a result, I've marked this for our next breaking change release.

I agree that the underlyingError change isn't absolutely necessary but it should be non-breaking and provide the underlying server Error in a more standard way.

[jurgowski]

I guess the main use case for CustomNSError is to make Swift errors easy to deal with in Obj-C code without custom bridging. It seems like in this case, the errors are coming from Obj-C and StorageError is a newly introduced error type (I didn't realize this when I first opened the issue).

I agree that I don't think there is a way to make a change here that doesn't alter current behavior since StorageError is an enum.. However, I would expect that anyone catching StorageError is probably catching all errors generically. It may be possible to use the error directly in the failure case of the Result in getResultCallback (without converting it to StorageError via swiftConvert). As long as StorageError isn't deleted, things would still compile.

[paulb777]

Some WIP at https://github.com/firebase/firebase-ios-sdk/commit/3fae8e7ebc364985642a1000e89b2f663b5fc8ae