Skip to content

Commit 6981eee

Browse files
authored
chore(cli-repl): return a cached promise from .close() MONGOSH-1943 (#2297)
This is mostly just making code a bit more resilient against cases in which we would unintentionally call `.close()` twice.
1 parent 43e1ca5 commit 6981eee

File tree

1 file changed

+46
-48
lines changed

1 file changed

+46
-48
lines changed

packages/cli-repl/src/cli-repl.ts

Lines changed: 46 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export class CliRepl implements MongoshIOProvider {
129129
toggleableAnalytics: ToggleableAnalytics = new ToggleableAnalytics();
130130
warnedAboutInaccessibleFiles = false;
131131
onExit: (code?: number) => Promise<never>;
132-
closing = false;
132+
closingPromise?: Promise<void>;
133133
isContainerizedEnvironment = false;
134134
hasOnDiskTelemetryId = false;
135135
proxyOptions: DevtoolsProxyOptions = {
@@ -1088,56 +1088,54 @@ export class CliRepl implements MongoshIOProvider {
10881088
* Close all open resources held by this REPL instance.
10891089
*/
10901090
async close(): Promise<void> {
1091-
markTime(TimingCategories.REPLInstantiation, 'start closing');
1092-
if (this.closing) {
1093-
return;
1094-
}
1095-
this.agent?.destroy();
1096-
if (!this.output.destroyed) {
1097-
// Wait for output to be fully flushed before exiting.
1098-
if (this.output.writableEnded) {
1099-
// .end() has been called but not finished; 'close' will be emitted in that case.
1100-
// (This should not typically happen in the context of mongosh, but there's also
1101-
// no reason not to handle this case properly.)
1102-
try {
1103-
await once(this.output, 'close');
1104-
} catch {
1105-
/* ignore */
1091+
return (this.closingPromise ??= (async () => {
1092+
markTime(TimingCategories.REPLInstantiation, 'start closing');
1093+
this.agent?.destroy();
1094+
if (!this.output.destroyed) {
1095+
// Wait for output to be fully flushed before exiting.
1096+
if (this.output.writableEnded) {
1097+
// .end() has been called but not finished; 'close' will be emitted in that case.
1098+
// (This should not typically happen in the context of mongosh, but there's also
1099+
// no reason not to handle this case properly.)
1100+
try {
1101+
await once(this.output, 'close');
1102+
} catch {
1103+
/* ignore */
1104+
}
1105+
} else {
1106+
// .end() has not been called; write an empty chunk and wait for it to be fully written.
1107+
await new Promise((resolve) => this.output.write('', resolve));
11061108
}
1107-
} else {
1108-
// .end() has not been called; write an empty chunk and wait for it to be fully written.
1109-
await new Promise((resolve) => this.output.write('', resolve));
11101109
}
1111-
}
1112-
markTime(TimingCategories.REPLInstantiation, 'output flushed');
1113-
this.closing = true;
1114-
const analytics = this.toggleableAnalytics;
1115-
let flushError: string | null = null;
1116-
let flushDuration: number | null = null;
1117-
if (analytics) {
1118-
const flushStart = Date.now();
1119-
try {
1120-
await analytics.flush();
1121-
markTime(TimingCategories.Telemetry, 'flushed analytics');
1122-
} catch (err: any) {
1123-
flushError = err.message;
1124-
} finally {
1125-
flushDuration = Date.now() - flushStart;
1126-
}
1127-
}
1128-
this.logWriter?.info(
1129-
'MONGOSH',
1130-
mongoLogId(1_000_000_045),
1131-
'analytics',
1132-
'Flushed outstanding data',
1133-
{
1134-
flushError,
1135-
flushDuration,
1110+
markTime(TimingCategories.REPLInstantiation, 'output flushed');
1111+
const analytics = this.toggleableAnalytics;
1112+
let flushError: string | null = null;
1113+
let flushDuration: number | null = null;
1114+
if (analytics) {
1115+
const flushStart = Date.now();
1116+
try {
1117+
await analytics.flush();
1118+
markTime(TimingCategories.Telemetry, 'flushed analytics');
1119+
} catch (err: any) {
1120+
flushError = err.message;
1121+
} finally {
1122+
flushDuration = Date.now() - flushStart;
1123+
}
11361124
}
1137-
);
1138-
await this.logWriter?.flush();
1139-
markTime(TimingCategories.Logging, 'flushed log writer');
1140-
this.bus.emit('mongosh:closed');
1125+
this.logWriter?.info(
1126+
'MONGOSH',
1127+
mongoLogId(1_000_000_045),
1128+
'analytics',
1129+
'Flushed outstanding data',
1130+
{
1131+
flushError,
1132+
flushDuration,
1133+
}
1134+
);
1135+
await this.logWriter?.flush();
1136+
markTime(TimingCategories.Logging, 'flushed log writer');
1137+
this.bus.emit('mongosh:closed');
1138+
})());
11411139
}
11421140

11431141
/**

0 commit comments

Comments
 (0)