Skip to content

Commit 4157b26

Browse files
authored
fix(NODE-7469): overload retry when retryReads/Writes=false (#4888)
1 parent aa8b39c commit 4157b26

File tree

6 files changed

+4334
-922
lines changed

6 files changed

+4334
-922
lines changed

src/operations/execute_operation.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,12 +389,28 @@ async function executeOperationWithRetries<
389389
);
390390

391391
function canRetry(operation: AbstractOperation, error: MongoError) {
392-
// always retryable
392+
// SystemOverloadedError is retryable, but must respect retryReads/retryWrites settings
393+
// Check topology options directly (not operation.canRetryRead/Write) because backpressure
394+
// expands retry support beyond traditional retryable reads/writes
395+
// NOTE: Unlike traditional retries, backpressure retries ARE allowed inside transactions
393396
if (
394397
error.hasErrorLabel(MongoErrorLabel.SystemOverloadedError) &&
395398
error.hasErrorLabel(MongoErrorLabel.RetryableError)
396399
) {
397-
return true;
400+
// runCommand requires BOTH retryReads and retryWrites to be enabled (per spec step 2.4)
401+
if (operation instanceof RunCommandOperation) {
402+
return topology.s.options.retryReads && topology.s.options.retryWrites;
403+
}
404+
405+
// Write-stage aggregates ($out/$merge) require retryWrites
406+
if (operation instanceof AggregateOperation && operation.hasWriteStage) {
407+
return topology.s.options.retryWrites;
408+
}
409+
410+
// For other operations, check if retries are enabled based on operation type
411+
const canRetryAsRead = hasReadAspect && topology.s.options.retryReads;
412+
const canRetryAsWrite = hasWriteAspect && topology.s.options.retryWrites;
413+
return canRetryAsRead || canRetryAsWrite;
398414
}
399415

400416
// run command is only retryable if we get retryable overload errors

test/integration/client-backpressure/client-backpressure.spec.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { type Test } from '../../tools/unified-spec-runner/schema';
44

55
const skippedTests = {
66
'collection.dropIndexes retries at most maxAttempts=5 times':
7+
'TODO(NODE-6517): dropIndexes squashes all errors other than ns not found',
8+
'collection.dropIndexes (write) does not retry if retryWrites=false':
79
'TODO(NODE-6517): dropIndexes squashes all errors other than ns not found'
810
};
911

0 commit comments

Comments
 (0)