Skip to content

Commit

Permalink
feat!: better error handling in SfCommand.catch (#568)
Browse files Browse the repository at this point in the history
* feat: better error handling in SfCommand.catch

* chore: bump to 11

* fix: handle oclif errors correctly

---------

Co-authored-by: Mike Donnalley <mdonnalley@salesforce.com>
  • Loading branch information
shetzel and mdonnalley committed Jun 13, 2024
1 parent 3a475da commit 93885ae
Show file tree
Hide file tree
Showing 9 changed files with 623 additions and 119 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@salesforce/sf-plugins-core",
"version": "10.0.1",
"version": "11.0.0",
"description": "Utils for writing Salesforce CLI plugins",
"main": "lib/exported",
"types": "lib/exported.d.ts",
Expand Down
109 changes: 109 additions & 0 deletions src/SfCommandError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* 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(err);
return new this({
message: sfError.message,
name: err.name ?? 'Error',
actions: 'actions' in err ? err.actions : undefined,
exitCode,
code: 'code' in err && err.code ? err.code : exitCode.toString(10),
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
30 changes: 3 additions & 27 deletions src/errorHandling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
*/

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

/**
*
Expand All @@ -21,7 +19,7 @@ import { removeEmpty } from './util.js';
* - use the process exitCode
* - default to 1
*/
export const computeErrorCode = (e: Error | SfError | SfCommandError): number => {
export const computeErrorCode = (e: Error | SfError | Errors.CLIError): number => {
// regardless of the exitCode, we'll set gacks and TypeError to a specific exit code
if (errorIsGack(e)) {
return 20;
Expand Down Expand Up @@ -66,28 +64,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 =>
const isOclifError = <T extends Error | SfError | Errors.CLIError>(e: T): e is T & Errors.CLIError =>
'oclif' in e ? true : false;
67 changes: 13 additions & 54 deletions src/sfCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import os from 'node:os';
import { Command, Config, HelpSection, Flags } from '@oclif/core';
import { Errors, Command, Config, HelpSection, Flags } from '@oclif/core';
import {
envVars,
Messages,
Expand All @@ -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> {
protected async catch(error: Error | SfError | Errors.CLIError): 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.

Loading

0 comments on commit 93885ae

Please sign in to comment.