-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat: updating not found exception to give more information #12813
base: master
Are you sure you want to change the base?
feat: updating not found exception to give more information #12813
Conversation
@bswhite1 - could you provide a code sample for the exception you're describing? Sounds like something we can test as well, it would be ideal to have an integration test 👌 . Thanks. |
Added some debugging to the function. This is the debug from the android sdk: This is the incoming platformException and it's fields: So given this data, the original returns: |
@bswhite1 - any chance you could write an integration test for this? Would be ideal to see if behaviour was the same across platforms. Thanks. |
Updated integration test. |
@bswhite1 - You need to fix the analyse issues (trailing commas in the test). Also - macOS and iOS e2e are failing on that test update which needs resolving 🙏 |
@russellwheatley I can't reproduce locally, so added some debug. If someone could start the workflow, that would be great. |
…nto not_found_exception_update
I was able to get it the workflows running locally. Actual android firestore: emulated android firestore: emulated macOS firestore: emulated iOS firestore: Should I wrap that test with a TargetPlatform.android check? Or is this indicating a bug in macOS / iOS? |
@russellwheatley Do you have a preference on how to proceed? |
Description
The standard exception was not very helpful. I could tell a file wasn't found, but no indication of why or what file. This uses the platformException.message that has the actual file path, and returns it to the user.
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. Updating the
pubspec.yaml
and changelogs is not required.///
).melos run analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?