Skip to content

Commit

Permalink
feat: better error handling in SfCommand.catch
Browse files Browse the repository at this point in the history
  • Loading branch information
shetzel committed Jun 11, 2024
1 parent af10787 commit c093c8f
Show file tree
Hide file tree
Showing 8 changed files with 578 additions and 113 deletions.
110 changes: 110 additions & 0 deletions src/SfCommandError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { SfError, StructuredMessage } from '@salesforce/core';
import { AnyJson } from '@salesforce/ts-types';
import { computeErrorCode } from './errorHandling.js';
import { removeEmpty } from './util.js';

// These types are 90% the same as SfErrorOptions (but they aren't exported to extend)
type ErrorDataProperties = AnyJson;
export type SfCommandErrorOptions<T extends ErrorDataProperties = ErrorDataProperties> = {
message: string;
exitCode: number;
code: string;
name?: string;
commandName: string;
data?: T;
cause?: unknown;
context?: string;
actions?: string[];
result?: unknown;
warnings?: Array<StructuredMessage | string>;
};

type SfCommandErrorJson = {
name: string;
message: string;
exitCode: number;
commandName: string;
context: string;
code: string;
status: string;
stack: string;
actions?: string;
data?: ErrorDataProperties;
cause?: string;
warnings?: Array<StructuredMessage | string>;
result?: unknown;
};

export class SfCommandError extends SfError {
public status: number;
public commandName: string;
public warnings?: Array<StructuredMessage | string>;
public result?: unknown;
public skipOclifErrorHandling: boolean;
public oclif: { exit: number };

/**
* SfCommandError is meant to wrap errors from `SfCommand.catch()` for a common
* error format to be logged, sent to telemetry, and re-thrown. Do not create
* instances from the constructor. Call the static method, `SfCommandError.from()`
* and use the returned `SfCommandError`.
*/
private constructor(input: SfCommandErrorOptions) {
super(input.message, input.name, input.actions, input.exitCode, input.cause);
this.data = input.data;
this.status = input.exitCode;
this.warnings = input.warnings;
this.skipOclifErrorHandling = true;
this.commandName = input.commandName;
this.code = input.code;
this.result = input.result;
this.oclif = { exit: input.exitCode };
this.context = input.context ?? input.commandName;
}

public static from(
err: Error | SfError | SfCommandError,
commandName: string,
warnings?: Array<StructuredMessage | string>
): SfCommandError {
// SfError.wrap() does most of what we want so start with that.
const sfError = SfError.wrap(err);
const exitCode = computeErrorCode(sfError);
sfError.code = 'code' in err ? err.code : exitCode.toString(10);
return new this({
message: sfError.message,
name: err.name ?? 'Error',
actions: 'actions' in err ? err.actions : undefined,
exitCode,
code: sfError.code,
cause: sfError.cause,
commandName: 'commandName' in err ? err.commandName : commandName,
data: 'data' in err ? err.data : undefined,
result: 'result' in err ? err.result : undefined,
context: 'context' in err ? err.context : commandName,
warnings,
});
}

public toJson(): SfCommandErrorJson {
return {
...removeEmpty({
// toObject() returns name, message, exitCode, actions, context, data
...this.toObject(),
stack: this.stack,
cause: this.cause,
warnings: this.warnings,
code: this.code,
status: this.status,
commandName: this.commandName,
result: this.result,
}),
} as SfCommandErrorJson;
}
}
7 changes: 4 additions & 3 deletions src/errorFormatting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { Mode, Messages, envVars } from '@salesforce/core';
import { inspect } from 'node:util';
import type { Ansis } from 'ansis';
import { Mode, Messages, envVars } from '@salesforce/core';
import { StandardColors } from './ux/standardColors.js';
import { SfCommandError } from './types.js';
import { SfCommandError } from './SfCommandError.js';

Messages.importMessagesDirectoryFromMetaUrl(import.meta.url);
const messages = Messages.loadMessages('@salesforce/sf-plugins-core', 'messages');
Expand Down Expand Up @@ -45,7 +46,7 @@ export const formatError = (error: SfCommandError): string =>
`${formatErrorPrefix(error)} ${error.message}`,
...formatActions(error.actions ?? []),
error.stack && envVars.getString('SF_ENV') === Mode.DEVELOPMENT
? StandardColors.info(`\n*** Internal Diagnostic ***\n\n${error.stack}\n******\n`)
? StandardColors.info(`\n*** Internal Diagnostic ***\n\n${inspect(error)}\n******\n`)
: [],
].join('\n');

Expand Down
25 changes: 1 addition & 24 deletions src/errorHandling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

import { SfError } from '@salesforce/core';
import { CLIError } from '@oclif/core/errors';
import { SfCommandError } from './types.js';
import { removeEmpty } from './util.js';
import { SfCommandError } from './SfCommandError.js';

/**
*
Expand Down Expand Up @@ -66,28 +65,6 @@ export const errorIsTypeError = (error: Error | SfError): boolean =>
Boolean(error.stack?.includes('TypeError')) ||
('cause' in error && error.cause instanceof Error && errorIsTypeError(error.cause));

export const errorToSfCommandError = (
codeFromError: number,
error: Error | SfError | SfCommandError,
commandName: string
): SfCommandError => ({
...removeEmpty({
code: codeFromError,
actions: 'actions' in error ? error.actions : null,
context: ('context' in error ? error.context : commandName) ?? commandName,
commandName: ('commandName' in error ? error.commandName : commandName) ?? commandName,
data: 'data' in error ? error.data : null,
result: 'result' in error ? error.result : null,
}),
...{
message: error.message,
name: error.name ?? 'Error',
status: codeFromError,
stack: error.stack,
exitCode: codeFromError,
},
});

/** custom typeGuard for handling the fact the SfCommand doesn't know about oclif error structure */
const isOclifError = <T extends Error | SfError | SfCommandError>(e: T): e is T & CLIError =>
'oclif' in e ? true : false;
63 changes: 11 additions & 52 deletions src/sfCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ import type { AnyJson } from '@salesforce/ts-types';
import { Progress } from './ux/progress.js';
import { Spinner } from './ux/spinner.js';
import { Ux } from './ux/ux.js';
import { SfCommandError } from './types.js';
import { SfCommandError } from './SfCommandError.js';
import { formatActions, formatError } from './errorFormatting.js';
import { StandardColors } from './ux/standardColors.js';
import { confirm, secretPrompt, PromptInputs } from './ux/prompts.js';
import { computeErrorCode, errorToSfCommandError } from './errorHandling.js';

Messages.importMessagesDirectoryFromMetaUrl(import.meta.url);
const messages = Messages.loadMessages('@salesforce/sf-plugins-core', 'messages');
Expand Down Expand Up @@ -201,7 +200,7 @@ export abstract class SfCommand<T> extends Command {

const colorizedArgs = [
`${StandardColors.warning(messages.getMessage('warning.prefix'))} ${message}`,
...formatActions(typeof input === 'string' ? [] : input.actions ?? [], { actionColor: StandardColors.info }),
...formatActions(typeof input === 'string' ? [] : input.actions ?? []),
];

this.logToStderr(colorizedArgs.join(os.EOL));
Expand All @@ -216,10 +215,9 @@ export abstract class SfCommand<T> extends Command {
public info(input: SfCommand.Info): void {
const message = typeof input === 'string' ? input : input.message;
this.log(
[
`${StandardColors.info(message)}`,
...formatActions(typeof input === 'string' ? [] : input.actions ?? [], { actionColor: StandardColors.info }),
].join(os.EOL)
[`${StandardColors.info(message)}`, ...formatActions(typeof input === 'string' ? [] : input.actions ?? [])].join(
os.EOL
)
);
}

Expand Down Expand Up @@ -368,66 +366,27 @@ export abstract class SfCommand<T> extends Command {
};
}

/**
* Wrap the command error into the standardized JSON structure.
*/
protected toErrorJson(error: SfCommand.Error): SfCommand.Error {
return {
...error,
warnings: this.warnings,
};
}

// eslint-disable-next-line @typescript-eslint/require-await
protected async catch(error: Error | SfError | SfCommand.Error): Promise<never> {
// stop any spinners to prevent it from unintentionally swallowing output.
// If there is an active spinner, it'll say "Error" instead of "Done"
this.spinner.stop(StandardColors.error('Error'));
// transform an unknown error into one that conforms to the interface

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
// const codeFromError = (error.oclif?.exit as number | undefined) ?? (error.exitCode as number | undefined) ?? 1;
const codeFromError = computeErrorCode(error);
process.exitCode = codeFromError;

const sfCommandError = errorToSfCommandError(codeFromError, error, this.statics.name);
// transform an unknown error into a SfCommandError
const sfCommandError = SfCommandError.from(error, this.statics.name, this.warnings);
process.exitCode = sfCommandError.exitCode;

if (this.jsonEnabled()) {
this.logJson(this.toErrorJson(sfCommandError));
this.logJson(sfCommandError.toJson());
} else {
this.logToStderr(formatError(sfCommandError));
}

// Create SfError that can be thrown
const err = new SfError(
error.message,
error.name ?? 'Error',
// @ts-expect-error because actions is not on Error
(error.actions as string[]) ?? [],
process.exitCode
);
if (sfCommandError.data) {
err.data = sfCommandError.data as AnyJson;
}
err.context = sfCommandError.context;
err.stack = sfCommandError.stack;
// @ts-expect-error because code is not on SfError
err.code = codeFromError;
// @ts-expect-error because status is not on SfError
err.status = sfCommandError.status;

// @ts-expect-error because skipOclifErrorHandling is not on SfError
err.skipOclifErrorHandling = true;

// Add oclif exit code to the error so that oclif can use the exit code when exiting.
// @ts-expect-error because oclif is not on SfError
err.oclif = { exit: process.exitCode };

// Emit an event for plugin-telemetry prerun hook to pick up.
// @ts-expect-error because TS is strict about the events that can be emitted on process.
process.emit('sfCommandError', err, this.id);
process.emit('sfCommandError', sfCommandError, this.id);

throw err;
throw sfCommandError;
}

public abstract run(): Promise<T>;
Expand Down
22 changes: 0 additions & 22 deletions src/types.ts

This file was deleted.

82 changes: 82 additions & 0 deletions test/unit/errorFormatting.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { expect } from 'chai';
import { Mode, SfError } from '@salesforce/core';
import { formatError } from '../../src/errorFormatting.js';
import { SfCommandError } from '../../src/SfCommandError.js';

describe('errorFormatting.formatError()', () => {
afterEach(() => {
delete process.env.SF_ENV;
});

it('should have correct output for non-development mode, no actions', () => {
const err = SfCommandError.from(new Error('this did not work'), 'thecommand');
const errorOutput = formatError(err);
expect(errorOutput).to.contain('Error (1)');
expect(errorOutput).to.contain('this did not work');
});

it('should have correct output for non-development mode, with actions', () => {
const sfError = new SfError('this did not work', 'BadError');
const err = SfCommandError.from(sfError, 'thecommand');
err.actions = ['action1', 'action2'];
const errorOutput = formatError(err);
expect(errorOutput).to.contain('Error (BadError)');
expect(errorOutput).to.contain('this did not work');
expect(errorOutput).to.contain('Try this:');
expect(errorOutput).to.contain('action1');
expect(errorOutput).to.contain('action2');
});

it('should have correct output for development mode, basic error', () => {
process.env.SF_ENV = Mode.DEVELOPMENT;
const err = SfCommandError.from(new SfError('this did not work'), 'thecommand');
const errorOutput = formatError(err);
expect(errorOutput).to.contain('Error (SfError)');
expect(errorOutput).to.contain('this did not work');
expect(errorOutput).to.contain('*** Internal Diagnostic ***');
expect(errorOutput).to.contain('at Function.from');
expect(errorOutput).to.contain('actions: undefined');
expect(errorOutput).to.contain('exitCode: 1');
expect(errorOutput).to.contain("context: 'thecommand'");
expect(errorOutput).to.contain('data: undefined');
expect(errorOutput).to.contain('cause: undefined');
expect(errorOutput).to.contain('status: 1');
expect(errorOutput).to.contain("commandName: 'thecommand'");
expect(errorOutput).to.contain('warnings: undefined');
expect(errorOutput).to.contain('result: undefined');
});

it('should have correct output for development mode, full error', () => {
process.env.SF_ENV = Mode.DEVELOPMENT;
const sfError = SfError.create({
name: 'WOMP_WOMP',
message: 'this did not work',
actions: ['action1', 'action2'],
cause: new Error('this is the cause'),
exitCode: 9,
context: 'somecommand',
data: { foo: 'bar' },
});
const err = SfCommandError.from(sfError, 'thecommand');
const errorOutput = formatError(err);
expect(errorOutput).to.contain('Error (WOMP_WOMP)');
expect(errorOutput).to.contain('this did not work');
expect(errorOutput).to.contain('*** Internal Diagnostic ***');
expect(errorOutput).to.contain('at Function.from');
expect(errorOutput).to.contain("actions: [ 'action1', 'action2' ]");
expect(errorOutput).to.contain('exitCode: 9');
expect(errorOutput).to.contain("context: 'somecommand'");
expect(errorOutput).to.contain("data: { foo: 'bar' }");
expect(errorOutput).to.contain('cause: Error: this is the cause');
expect(errorOutput).to.contain('status: 9');
expect(errorOutput).to.contain("commandName: 'thecommand'");
expect(errorOutput).to.contain('warnings: undefined');
expect(errorOutput).to.contain('result: undefined');
});
});
Loading

0 comments on commit c093c8f

Please sign in to comment.