fix(logging): fix log flushing on exit MONGOSH-2883

[addaleax]

Depends on mongodb-js/devtools-shared#578

[Copilot]

Pull Request Overview

This PR fixes log flushing on exit by making the logging flush operations asynchronous. The changes ensure that log files are properly written before the application terminates, preventing potential data loss.

• Changed flush() and detachLogger() methods to return Promise<void> instead of void
• Added await calls for proper asynchronous handling of log flushing operations
• Introduced error handling in the start() method to ensure cleanup occurs even when exceptions are thrown

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
packages/logging/src/types.ts	Updated interface to make flush and detachLogger methods async
packages/logging/src/logging-and-telemetry.ts	Implemented async flush behavior with proper log.flush() awaiting
packages/e2e-tests/test/e2e.spec.ts	Added tests to verify log flushing works on normal exit and exception scenarios
packages/cli-repl/src/cli-repl.ts	Updated calls to use async flush methods and added error handling wrapper

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The error message filtering using regex replacement is fragile and could inadvertently remove legitimate log content. Consider a more robust approach to extract the log path from shell output.

Suggested change

	const log = await readReplLogFile(
	shell.output.replace(/Error: uh oh/g, '').trim()
	);
	// Extract the log path from the shell output by finding the line that looks like a log file path
	const logPathLine = shell.output
	.split('\n')
	.map(line => line.trim())
	.find(line => line.length > 0 && /\.log$/.test(line));
	expect(logPathLine, 'Could not find log path in shell output').to.be.a('string');
	const log = await readReplLogFile(logPathLine);

[addaleax]

I mean, I had similar thoughts here as well, although I was ultimately feeling like this is just fine for a test

[gagik]

for future, we should probably figure out why we don't have a lint rule for this

[addaleax]

@gagik We do 🙂 This is because I changed the type of .detachLogger() to return a Promise in this PR, too (in fact, the lint rule helped with catching all of these cases)

[gagik]

ah yes