Skip to content

Commit 69aef52

Browse files
committed
chore: fix more
1 parent 53e40e4 commit 69aef52

3 files changed

Lines changed: 61 additions & 45 deletions

File tree

src/cmap/auth/mongodb_aws.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import type { Binary, BSONSerializeOptions } from '../../bson';
22
import * as BSON from '../../bson';
3-
import { aws4 } from '../../deps';
3+
import { type AWS4, loadAws4 } from '../../deps';
44
import {
55
MongoCompatibilityError,
66
MongoMissingCredentialsError,
7+
type MongoMissingDependencyError,
78
MongoRuntimeError
89
} from '../../error';
910
import { ByteUtils, maxWireVersion, ns, randomBytes } from '../../utils';
@@ -32,6 +33,12 @@ interface AWSSaslContinuePayload {
3233
t?: string;
3334
}
3435

36+
let aws4:
37+
| AWS4
38+
| {
39+
kModuleError: MongoMissingDependencyError;
40+
};
41+
3542
export class MongoDBAWS extends AuthProvider {
3643
private credentialFetcher: AWSTemporaryCredentialProvider;
3744
constructor() {
@@ -48,6 +55,7 @@ export class MongoDBAWS extends AuthProvider {
4855
throw new MongoMissingCredentialsError('AuthContext must provide credentials.');
4956
}
5057

58+
aws4 ??= loadAws4();
5159
if ('kModuleError' in aws4) {
5260
throw aws4['kModuleError'];
5361
}

src/cursor/abstract_cursor.ts

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,50 +1146,59 @@ class ReadableCursorStream extends Readable {
11461146
return;
11471147
}
11481148

1149-
this._cursor.next().then(
1150-
result => {
1151-
if (result == null) {
1152-
this.push(null);
1153-
} else if (this.destroyed) {
1154-
this._cursor.close().then(undefined, squashError);
1155-
} else {
1156-
if (this.push(result)) {
1157-
return this._readNext();
1149+
this._cursor
1150+
.next()
1151+
.then(
1152+
// result from next()
1153+
result => {
1154+
if (result == null) {
1155+
this.push(null);
1156+
} else if (this.destroyed) {
1157+
this._cursor.close().then(undefined, squashError);
1158+
} else {
1159+
if (this.push(result)) {
1160+
return this._readNext();
1161+
}
1162+
1163+
this._readInProgress = false;
1164+
}
1165+
},
1166+
// error from next()
1167+
err => {
1168+
// NOTE: This is questionable, but we have a test backing the behavior. It seems the
1169+
// desired behavior is that a stream ends cleanly when a user explicitly closes
1170+
// a client during iteration. Alternatively, we could do the "right" thing and
1171+
// propagate the error message by removing this special case.
1172+
if (err.message.match(/server is closed/)) {
1173+
this._cursor.close().then(undefined, squashError);
1174+
return this.push(null);
11581175
}
11591176

1160-
this._readInProgress = false;
1161-
}
1162-
},
1163-
err => {
1164-
// NOTE: This is questionable, but we have a test backing the behavior. It seems the
1165-
// desired behavior is that a stream ends cleanly when a user explicitly closes
1166-
// a client during iteration. Alternatively, we could do the "right" thing and
1167-
// propagate the error message by removing this special case.
1168-
if (err.message.match(/server is closed/)) {
1169-
this._cursor.close().then(undefined, squashError);
1170-
return this.push(null);
1171-
}
1177+
// NOTE: This is also perhaps questionable. The rationale here is that these errors tend
1178+
// to be "operation was interrupted", where a cursor has been closed but there is an
1179+
// active getMore in-flight. This used to check if the cursor was killed but once
1180+
// that changed to happen in cleanup legitimate errors would not destroy the
1181+
// stream. There are change streams test specifically test these cases.
1182+
if (err.message.match(/operation was interrupted/)) {
1183+
return this.push(null);
1184+
}
11721185

1173-
// NOTE: This is also perhaps questionable. The rationale here is that these errors tend
1174-
// to be "operation was interrupted", where a cursor has been closed but there is an
1175-
// active getMore in-flight. This used to check if the cursor was killed but once
1176-
// that changed to happen in cleanup legitimate errors would not destroy the
1177-
// stream. There are change streams test specifically test these cases.
1178-
if (err.message.match(/operation was interrupted/)) {
1179-
return this.push(null);
1186+
// NOTE: The two above checks on the message of the error will cause a null to be pushed
1187+
// to the stream, thus closing the stream before the destroy call happens. This means
1188+
// that either of those error messages on a change stream will not get a proper
1189+
// 'error' event to be emitted (the error passed to destroy). Change stream resumability
1190+
// relies on that error event to be emitted to create its new cursor and thus was not
1191+
// working on 4.4 servers because the error emitted on failover was "interrupted at
1192+
// shutdown" while on 5.0+ it is "The server is in quiesce mode and will shut down".
1193+
// See NODE-4475.
1194+
return this.destroy(err);
11801195
}
1181-
1182-
// NOTE: The two above checks on the message of the error will cause a null to be pushed
1183-
// to the stream, thus closing the stream before the destroy call happens. This means
1184-
// that either of those error messages on a change stream will not get a proper
1185-
// 'error' event to be emitted (the error passed to destroy). Change stream resumability
1186-
// relies on that error event to be emitted to create its new cursor and thus was not
1187-
// working on 4.4 servers because the error emitted on failover was "interrupted at
1188-
// shutdown" while on 5.0+ it is "The server is in quiesce mode and will shut down".
1189-
// See NODE-4475.
1190-
return this.destroy(err);
1191-
}
1192-
);
1196+
)
1197+
// if either of the above handlers throw
1198+
.catch(error => {
1199+
this._readInProgress = false;
1200+
this.destroy(error);
1201+
});
11931202
}
11941203
}
11951204

src/deps.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ export function getSocks(): SocksLib | { kModuleError: MongoMissingDependencyErr
203203
}
204204
}
205205

206-
interface AWS4 {
206+
/** @internal */
207+
export interface AWS4 {
207208
/**
208209
* Created these inline types to better assert future usage of this API
209210
* @param options - options for request
@@ -244,9 +245,7 @@ interface AWS4 {
244245
};
245246
}
246247

247-
export const aws4: AWS4 | { kModuleError: MongoMissingDependencyError } = loadAws4();
248-
249-
function loadAws4() {
248+
export function loadAws4() {
250249
let aws4: AWS4 | { kModuleError: MongoMissingDependencyError };
251250
try {
252251
// eslint-disable-next-line @typescript-eslint/no-require-imports

0 commit comments

Comments
 (0)