这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[firebase_storage] Return error when failing to read file #1504

Merged
merged 4 commits into from
Apr 29, 2019

Conversation

LinusU
Copy link
Contributor

@LinusU LinusU commented Apr 17, 2019

Description

Currently, when trying to upload a File that doesn't exist, the entire app crashes deep inside the firebase sdk: https://github.com/firebase/firebase-ios-sdk/pull/2458/files#r276273826

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

Not sure how to add tests since the tests since to mock out the Objective-C part of the plugins, any help would be appreciated!

@collinjackson
Copy link
Contributor

collinjackson commented Apr 18, 2019

Integration tests are being added in #1513, once those are landed we can add a test for this.

@collinjackson
Copy link
Contributor

Added an assertion on the Dart side since the current API doesn't provide a way to catch this exception. I think that this situation could be improved in the future, but also I don't see a downside to adding extra error handling for the missing file case. @kroikie do you think we should go ahead and merge this one?

@collinjackson
Copy link
Contributor

collinjackson commented Apr 29, 2019

Additional error handling seems like an improvement on the current implementation so I'll go ahead and merge it.

@collinjackson collinjackson merged commit 1009cd7 into flutter:master Apr 29, 2019
@LinusU
Copy link
Contributor Author

LinusU commented Apr 30, 2019

Thanks for moving forward with this @collinjackson, been away for the past days ❤️

@LinusU LinusU deleted the patch-1 branch April 30, 2019 12:39
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
[firebase_storage] Return error when failing to read file and assert that file exists when calling putFile
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
* Move cloud_firestore to cloud_firestore/cloud_firestore. This is required for federated implementations.
* Update docs, version bump to 0.12.10+4
* Fix analysis_options.yaml
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants