Skip to content

Commit 0c26ee5

Browse files
authored
feat: make bug reporting information more actionable MONGOSH-459 (#1043)
- Remove the additional text from the error message itself and instead move that responsibility to the output formatter - This makes sense, because this is not actually part of the error condition, and it allows different display methods to adjust the error output (e.g. include a log file if present, make the link clickable, refer to another JIRA project if appropriate, etc.) - Add the path to the log file that the reporter would ideally include in the CLI package.
1 parent edc5e77 commit 0c26ee5

File tree

8 files changed

+48
-8
lines changed

8 files changed

+48
-8
lines changed

packages/browser-repl/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
"@leafygreen-ui/palette": "^2.0.0",
3737
"@leafygreen-ui/syntax": "^2.2.0",
3838
"@mongosh/browser-runtime-core": "0.0.0-dev.0",
39+
"@mongosh/errors": "0.0.0-dev.0",
3940
"@mongosh/history": "0.0.0-dev.0",
4041
"@mongosh/i18n": "0.0.0-dev.0",
4142
"@mongosh/node-runtime-worker-thread": "0.0.0-dev.0",

packages/browser-repl/src/components/types/error-output.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React, { Component } from 'react';
22
import PropTypes from 'prop-types';
3+
import { isShouldReportAsBugError } from '@mongosh/errors';
34

45
import { SimpleTypeOutput } from './simple-type-output';
56
import { Expandable } from '../utils/expandable';
@@ -26,6 +27,16 @@ export class ErrorOutput extends Component<ErrorOutputProps> {
2627
return this.props.value.stack.split('\n').slice(1).join('\n');
2728
}
2829

30+
formatErrorBugReportInfo(): JSX.Element | undefined {
31+
if (isShouldReportAsBugError(this.props.value)) {
32+
return (<div>
33+
This is an error inside mongosh.
34+
Please <a href="https://jira.mongodb.org/projects/MONGOSH/issues" target="_blank">file a bug report for the MONGOSH project</a>.
35+
</div>);
36+
}
37+
return undefined;
38+
}
39+
2940
formatErrorInfo(): JSX.Element | undefined {
3041
if (this.props.value.errInfo) {
3142
return (<div>
@@ -50,6 +61,7 @@ export class ErrorOutput extends Component<ErrorOutputProps> {
5061
return (<div>
5162
{this.renderCollapsed(toggle)}
5263
<div>
64+
{this.formatErrorBugReportInfo()}
5365
{this.formatErrorInfo()}
5466
{this.formatErrorResult()}
5567
<pre>{this.formatStack()}</pre>

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,18 +276,21 @@ class CliRepl {
276276
}
277277
}
278278

279+
get logFilePath(): string {
280+
return this.shellHomeDirectory.localPath(`${this.logId}_log`);
281+
}
282+
279283
/**
280284
* Open a writable stream for the current log file.
281285
*/
282286
async openLogStream(): Promise<Writable> {
283-
const path = this.shellHomeDirectory.localPath(`${this.logId}_log`);
284287
await this.cleanupOldLogfiles();
285288
try {
286-
const stream = createWriteStream(path, { mode: 0o600 });
289+
const stream = createWriteStream(this.logFilePath, { mode: 0o600 });
287290
await once(stream, 'ready');
288291
return stream;
289292
} catch (err) {
290-
this.warnAboutInaccessibleFile(err, path);
293+
this.warnAboutInaccessibleFile(err, this.logFilePath);
291294
return new Writable({
292295
write(chunk, enc, cb) {
293296
// Just ignore log data if there was an error.
@@ -492,6 +495,10 @@ class CliRepl {
492495
throw e;
493496
}
494497
}
498+
499+
bugReportErrorMessageInfo(): string {
500+
return `Please include the log file for this session (${this.logFilePath}).`;
501+
}
495502
}
496503

497504
export default CliRepl;

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import util from 'util';
77
import stripAnsi from 'strip-ansi';
88
import clr from './clr';
99
import { HelpProperties, CollectionNamesWithTypes } from '@mongosh/shell-api';
10+
import { isShouldReportAsBugError } from '@mongosh/errors';
1011

1112
type EvaluationResult = {
1213
value: any;
@@ -20,6 +21,7 @@ type FormatOptions = {
2021
maxArrayLength?: number;
2122
maxStringLength?: number;
2223
showStackTraces?: boolean;
24+
bugReportErrorMessageInfo?: string;
2325
};
2426

2527
/**
@@ -178,6 +180,12 @@ export function formatError(error: Error, options: FormatOptions): string {
178180
let result = '';
179181
if (error.name) result += `\r${clr(error.name, ['bold', 'red'], options)}: `;
180182
if (error.message) result += error.message;
183+
if (isShouldReportAsBugError(error)) {
184+
result += '\nThis is an error inside mongosh. Please file a bug report for the MONGOSH project here: https://jira.mongodb.org/projects/MONGOSH/issues.';
185+
if (options.bugReportErrorMessageInfo) {
186+
result += `\n${options.bugReportErrorMessageInfo}`;
187+
}
188+
}
181189
if (error.name === 'SyntaxError') {
182190
if (!options.colors) {
183191
// Babel applies syntax highlighting to its errors by default.

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export type MongoshIOProvider = Omit<ConfigProvider<CliUserConfig>, 'validateCon
3131
exit(code?: number): Promise<never>;
3232
readFileUTF8(filename: string): Promise<{ contents: string, absolutePath: string }>;
3333
startMongocryptd(): Promise<AutoEncryptionOptions['extraOptions']>;
34+
bugReportErrorMessageInfo?(): string | undefined;
3435
};
3536

3637
export type MongoshNodeReplOptions = {
@@ -599,14 +600,15 @@ class MongoshNodeRepl implements EvaluationListener {
599600
return clr(text, style, this.getFormatOptions());
600601
}
601602

602-
getFormatOptions(): { colors: boolean, compact: number | boolean, depth: number, showStackTraces: boolean } {
603+
getFormatOptions(): { colors: boolean, compact: number | boolean, depth: number, showStackTraces: boolean, bugReportErrorMessageInfo?: string } {
603604
const output = this.output as WriteStream;
604605
return {
605606
colors: this._runtimeState?.repl?.useColors ??
606607
(output.isTTY && output.getColorDepth() > 1),
607608
compact: this.inspectCompact,
608609
depth: this.inspectDepth,
609-
showStackTraces: this.showStackTraces
610+
showStackTraces: this.showStackTraces,
611+
bugReportErrorMessageInfo: this.ioProvider.bugReportErrorMessageInfo?.()
610612
};
611613
}
612614

packages/cli-repl/test/e2e.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,12 @@ describe('e2e', function() {
125125
shell.writeInputLine('process.exitCode = 42; quit()');
126126
expect(await onExit).to.equal(42);
127127
});
128+
it('decorates internal errors with bug reporting information', async() => {
129+
const err = await shell.executeLine('throw Object.assign(new Error("foo"), { code: "COMMON-90001" })');
130+
expect(err).to.match(/^Error: foo$/m);
131+
expect(err).to.match(/^This is an error inside mongosh\. Please file a bug report for the MONGOSH project here: https:\/\/jira.mongodb.org\/projects\/MONGOSH\/issues\.$/m);
132+
expect(err).to.match(/^Please include the log file for this session \(.+[/\\][a-f0-9]{24}_log\)\.$/m);
133+
});
128134
});
129135
describe('set db', () => {
130136
for (const { mode, dbname, dbnameUri } of [

packages/errors/src/index.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ describe('errors', () => {
2626
const error = new MongoshInternalError('Something went wrong.');
2727
expect(error).to.be.instanceOf(MongoshBaseError);
2828
expect(error.name).to.be.equal('MongoshInternalError');
29-
expect(error.message).to.be.equal('[COMMON-90001] Something went wrong.\nThis is an error inside mongosh. Please file a bug report for the MONGOSH project here: https://jira.mongodb.org/projects/MONGOSH/issues.');
29+
expect(error.message).to.be.equal('[COMMON-90001] Something went wrong.');
3030
expect(error.code).to.be.equal(CommonErrors.UnexpectedInternalError);
3131
expect(error.scope).to.be.equal('COMMON');
3232
});

packages/errors/src/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ function getScopeFromErrorCode(code: string | null | undefined): string | undefi
88
return !match ? undefined : match[1];
99
}
1010

11+
function isShouldReportAsBugError(err: Error & { code?: string }): boolean {
12+
return err?.code === CommonErrors.UnexpectedInternalError;
13+
}
14+
1115
abstract class MongoshBaseError extends Error {
1216
readonly code: string | undefined;
1317
readonly scope: string | undefined;
@@ -32,8 +36,7 @@ class MongoshInternalError extends MongoshBaseError {
3236
constructor(message: string, metadata?: Object) {
3337
super(
3438
'MongoshInternalError',
35-
`${message}
36-
This is an error inside mongosh. Please file a bug report for the MONGOSH project here: https://jira.mongodb.org/projects/MONGOSH/issues.`,
39+
message,
3740
CommonErrors.UnexpectedInternalError,
3841
metadata
3942
);
@@ -82,6 +85,7 @@ class MongoshCommandFailed extends MongoshBaseError {
8285

8386
export {
8487
getScopeFromErrorCode,
88+
isShouldReportAsBugError,
8589
MongoshBaseError,
8690
MongoshWarning,
8791
MongoshRuntimeError,

0 commit comments

Comments
 (0)