Skip to content

Commit 5dbdfbf

Browse files
committed
Treat ABORTED writes as retryable
This corresponds to a server-side change that will be released for the v1 protocol only (see b/119437764). This is a port of firebase/firebase-js-sdk#1456.
1 parent 841657b commit 5dbdfbf

File tree

2 files changed

+27
-6
lines changed

2 files changed

+27
-6
lines changed

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/Datastore.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,13 @@ public Task<List<MaybeDocument>> lookup(List<DocumentKey> keys) {
219219
});
220220
}
221221

222-
public static boolean isPermanentWriteError(Status status) {
222+
/**
223+
* Determines whether the given status has an error code that represents a permanent error when
224+
* received in response to a non-write operation.
225+
*
226+
* @see #isPermanentWriteError for classifying write errors.
227+
*/
228+
public static boolean isPermanentError(Status status) {
223229
// See go/firestore-client-errors
224230
switch (status.getCode()) {
225231
case OK:
@@ -251,4 +257,19 @@ public static boolean isPermanentWriteError(Status status) {
251257
throw new IllegalArgumentException("Unknown gRPC status code: " + status.getCode());
252258
}
253259
}
260+
261+
/**
262+
* Determines whether the given status has an error code represents a permanent error when
263+
* received in response to a write operation.
264+
*
265+
* <p>Write operations must be handled specially because as of b/119437764, ABORTED errors on the
266+
* write stream should be retried too (even though ABORTED errors are not generally retryable).
267+
*
268+
* <p>Note that during the initial handshake on the write stream an ABORTED error signals that we
269+
* should discard our stream token (i.e. it is permanent). This means a handshake error should be
270+
* classified with isPermanentError, above.
271+
*/
272+
public static boolean isPermanentWriteError(Status status) {
273+
return isPermanentError(status) && !status.getCode().equals(Status.Code.ABORTED);
274+
}
254275
}

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteStore.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import com.google.firebase.firestore.util.Util;
3838
import com.google.protobuf.ByteString;
3939
import io.grpc.Status;
40-
import io.grpc.Status.Code;
4140
import java.util.ArrayDeque;
4241
import java.util.Deque;
4342
import java.util.HashMap;
@@ -642,9 +641,10 @@ private void handleWriteStreamClose(Status status) {
642641

643642
private void handleWriteHandshakeError(Status status) {
644643
hardAssert(!status.isOk(), "Handling write error with status OK.");
645-
// Reset the token if it's a permanent error or the error code is ABORTED, signaling the write
646-
// stream is no longer valid.
647-
if (Datastore.isPermanentWriteError(status) || status.getCode().equals(Code.ABORTED)) {
644+
// Reset the token if it's a permanent error, signaling the write stream is no longer valid.
645+
// Note that the handshake does not count as a write: see comments on isPermanentWriteError for
646+
// details.
647+
if (Datastore.isPermanentError(status)) {
648648
String token = Util.toDebugString(writeStream.getLastStreamToken());
649649
Logger.debug(
650650
LOG_TAG,
@@ -658,7 +658,7 @@ private void handleWriteHandshakeError(Status status) {
658658

659659
private void handleWriteError(Status status) {
660660
hardAssert(!status.isOk(), "Handling write error with status OK.");
661-
// Only handle permanent error, if it's transient just let the retry logic kick in.
661+
// Only handle permanent errors here. If it's transient, just let the retry logic kick in.
662662
if (Datastore.isPermanentWriteError(status)) {
663663
// If this was a permanent error, the request itself was the problem so it's not going
664664
// to succeed if we resend it.

0 commit comments

Comments
 (0)