Skip to content

Commit

Permalink
fix: make retryability async and handle PoolClearedError
Browse files Browse the repository at this point in the history
  • Loading branch information
dariakp committed Aug 26, 2022
1 parent 27c6327 commit b696513
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 18 deletions.
34 changes: 19 additions & 15 deletions src/operations/execute_operation.ts
@@ -1,3 +1,5 @@
import { setTimeout } from 'timers';

import type { Document } from '../bson';
import {
isRetryableReadError,
Expand Down Expand Up @@ -224,23 +226,25 @@ function executeWithServerSelection<TResult>(
}

// select a new server, and attempt to retry the operation
topology.selectServer(selector, serverSelectionOptions, (error?: Error, server?: Server) => {
if (!error && isWriteOperation && !supportsRetryableWrites(server)) {
return callback(
new MongoUnexpectedServerResponseError(
'Selected server does not support retryable writes'
)
);
}
setTimeout(() => {
topology.selectServer(selector, serverSelectionOptions, (error?: Error, server?: Server) => {
if (!error && isWriteOperation && !supportsRetryableWrites(server)) {
return callback(
new MongoUnexpectedServerResponseError(
'Selected server does not support retryable writes'
)
);
}

if (error || !server) {
return callback(
error ?? new MongoUnexpectedServerResponseError('Server selection failed without error')
);
}
if (error || !server) {
return callback(
error ?? new MongoUnexpectedServerResponseError('Server selection failed without error')
);
}

operation.execute(server, session, callback);
});
operation.execute(server, session, callback);
});
}, 1);
}

if (
Expand Down
7 changes: 6 additions & 1 deletion src/sdam/server.ts
Expand Up @@ -5,6 +5,7 @@ import {
ConnectionPoolEvents,
ConnectionPoolOptions
} from '../cmap/connection_pool';
import { PoolClearedError } from '../cmap/errors';
import {
APM_EVENTS,
CLOSED,
Expand Down Expand Up @@ -358,7 +359,11 @@ export class Server extends TypedEventEmitter<ServerEvents> {
(err, conn, cb) => {
if (err || !conn) {
this.s.operationCount -= 1;
markServerUnknown(this, err);
if (!(err instanceof PoolClearedError)) {
markServerUnknown(this, err);
} else {
err.addErrorLabel(MongoErrorLabel.RetryableWriteError);
}
return cb(err);
}

Expand Down
Expand Up @@ -19,8 +19,6 @@ const filter: TestFilter = ({ description }) => {
return isAuthEnabled
? 'TODO(NODE-3135): handle auth errors, also see NODE-3891: fix tests broken when AUTH enabled'
: false;
case 'PoolClearedError does not mark server unknown':
return 'TODO(NODE-3135): make CMAP SDAM-aware and ensure PoolClearError is retryable';
default:
return false;
}
Expand Down
1 change: 1 addition & 0 deletions test/tools/unified-spec-runner/entities.ts
Expand Up @@ -78,6 +78,7 @@ export class UnifiedThread {
this.#killed = true;
await this.#promise;
if (this.#error) {
this.#error.message = `<Thread(${this.id})>: ${this.#error.message}`;
throw this.#error;
}
}
Expand Down

0 comments on commit b696513

Please sign in to comment.