diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md index f9ea9ff2c31..e768fc49283 100644 --- a/firebase-firestore/CHANGELOG.md +++ b/firebase-firestore/CHANGELOG.md @@ -4,6 +4,9 @@ - [changed] Changed `get()` to only make 1 attempt to reach the backend before returning cached data, potentially reducing delays while offline. Previously it would make 2 attempts, to work around a backend bug. +- [changed] Some SDK errors that represent common mistakes (such as permission + denied or a missing index) will automatically be logged as a warning in + addition to being surfaced via the API. # 17.1.0 - [feature] Added `FieldValue.arrayUnion()` and `FieldValue.arrayRemove()` to diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java index 2f42ebf92e3..3bb60e8d4ca 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java @@ -377,9 +377,11 @@ public void handleRejectedListen(int targetId, Status error) { } else { QueryView queryView = queryViewsByTarget.get(targetId); hardAssert(queryView != null, "Unknown target: %s", targetId); - localStore.releaseQuery(queryView.getQuery()); + Query query = queryView.getQuery(); + localStore.releaseQuery(query); removeAndCleanup(queryView); - callback.onError(queryView.getQuery(), error); + logErrorIfInteresting(error, "Listen for %s failed", query); + callback.onError(query, error); } } @@ -401,13 +403,17 @@ public void handleSuccessfulWrite(MutationBatchResult mutationBatchResult) { @Override public void handleRejectedWrite(int batchId, Status status) { assertCallback("handleRejectedWrite"); + ImmutableSortedMap changes = localStore.rejectBatch(batchId); + + if (!changes.isEmpty()) { + logErrorIfInteresting(status, "Write failed at %s", changes.getMinKey().getPath()); + } // The local store may or may not be able to apply the write result and raise events immediately // (depending on whether the watcher is caught up), so we raise user callbacks first so that // they consistently happen before listen events. notifyUser(batchId, status); - ImmutableSortedMap changes = localStore.rejectBatch(batchId); emitNewSnapshot(changes, /*remoteEvent=*/ null); } @@ -551,4 +557,28 @@ public void handleCredentialChange(User user) { // Notify remote store so it can restart its streams. remoteStore.handleCredentialChange(); } + + /** + * Logs the error as a warnings if it likely represents a developer mistake such as forgetting to + * create an index or permission denied. + */ + private void logErrorIfInteresting(Status error, String contextString, Object... contextArgs) { + if (errorIsInteresting(error)) { + String context = String.format(contextString, contextArgs); + Logger.warn("Firestore", "%s: %s", context, error); + } + } + + private boolean errorIsInteresting(Status error) { + Status.Code code = error.getCode(); + String description = error.getDescription() != null ? error.getDescription() : ""; + + if (code == Status.Code.FAILED_PRECONDITION && description.contains("requires an index")) { + return true; + } else if (code == Status.Code.PERMISSION_DENIED) { + return true; + } + + return false; + } }