flutterfire icon indicating copy to clipboard operation
flutterfire copied to clipboard

feat: updating not found exception to give more information

Open bswhite1 opened this issue 1 year ago • 8 comments

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.

  • [X] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • [X] All existing and new tests are passing.
  • [X] I updated/added relevant documentation (doc comments with ///).
  • [X] The analyzer (melos run analyze) does not report any problems on my PR.
  • [X] I read and followed the Flutter Style Guide.
  • [X] I signed the CLA.
  • [X] 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.
  • [X] No, this is not a breaking change.

bswhite1 avatar May 21 '24 20:05 bswhite1

@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.

russellwheatley avatar May 22 '24 09:05 russellwheatley

Added some debugging to the function.

This is the debug from the android sdk: W/Firestore(18105): (24.11.0) [WriteStream]: (a26a33) Stream closed with status: Status{code=NOT_FOUND, description=No document to update: projects/project-name/databases/(default)/documents/CollectionA/DocumentB/CollectionC/missing_document, cause=null}.

This is the incoming platformException and it's fields: I/flutter (18105): platformException.code : firebase_firestore I/flutter (18105): platformException.details: {code: not-found, message: Some requested document was not found.} I/flutter (18105): platformException.message: com.google.firebase.firestore.FirebaseFirestoreException: NOT_FOUND: No document to update: projects/project-name/databases/(default)/documents/CollectionA/DocumentB/CollectionC/missing_document

So given this data, the original returns: Some requested document was not found. and this would return No document to update: projects/project-name/databases/(default)/documents/CollectionA/DocumentB/CollectionC/missing_document

bswhite1 avatar May 22 '24 15:05 bswhite1

@bswhite1 - any chance you could write an integration test for this? Would be ideal to see if behaviour was the same across platforms. Thanks.

russellwheatley avatar Jun 28 '24 07:06 russellwheatley

Updated integration test.

bswhite1 avatar Jul 10 '24 17:07 bswhite1

@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 avatar Jul 26 '24 08:07 russellwheatley

@russellwheatley I can't reproduce locally, so added some debug. If someone could start the workflow, that would be great. https://github.com/firebase/flutterfire/actions/runs/10113353206

bswhite1 avatar Jul 26 '24 15:07 bswhite1

I was able to get it the workflows running locally. Seems to be some inconsistency between actual android firestore, emulated android firestore, emulated macOS firestore, and emulated ios firestore.

Actual android firestore: platformException.code : firebase_firestore platformException.details: {code: not-found, message: Some requested document was not found.} platformException.message: com.google.firebase.firestore.FirebaseFirestoreException: NOT_FOUND: No document to update: projects/project-name/databases/(default)/documents/CollectionA/DocumentB/CollectionC/missing_document

emulated android firestore: platformException.code: firebase_firestore platformException.details: {code: not-found, message: Some requested document was not found.} platformException.message: com.google.firebase.firestore.FirebaseFirestoreException: NOT_FOUND: no entity to update: app: "dev~flutterfire-e2e-tests"

emulated macOS firestore: platformException.code: not-found platformException.details: {message: Some requested document was not found., code: not-found} platformException.message: Some requested document was not found.

emulated iOS firestore: platformException.code: not-found platformException.details: {message: Some requested document was not found., code: not-found} platformException.message: Some requested document was not found.

Should I wrap that test with a TargetPlatform.android check? Or is this indicating a bug in macOS / iOS?

bswhite1 avatar Jul 29 '24 16:07 bswhite1

@russellwheatley Do you have a preference on how to proceed?

bswhite1 avatar Aug 12 '24 13:08 bswhite1