Skip to content
This repository has been archived by the owner on Aug 22, 2023. It is now read-only.

feat: add oclif pretty printer #27

Merged
merged 15 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 19 additions & 18 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,31 @@
"author": "Jeff Dickey @jdxcode",
"bugs": "https://github.com/oclif/errors/issues",
"dependencies": {
"clean-stack": "^2.0.0",
"fs-extra": "^7.0.0",
"indent-string": "^3.2.0",
"clean-stack": "^3.0.0",
"fs-extra": "^9.0.1",
"indent-string": "^4.0.0",
"strip-ansi": "^6.0.0",
"wrap-ansi": "^4.0.0"
"wrap-ansi": "^7.0.0"
},
"devDependencies": {
"@types/chai": "^4.1.6",
"@types/clean-stack": "^1.3.0",
"@types/fs-extra": "^5.0.4",
"@types/indent-string": "^3.0.0",
"@types/mocha": "^5.2.5",
"@types/node": "^10.11.7",
"@types/strip-ansi": "^3.0.0",
"@types/chai": "^4.2.11",
"@types/clean-stack": "^2.1.1",
"@types/fs-extra": "^9.0.1",
"@types/indent-string": "^4.0.1",
"@types/mocha": "^7.0.2",
"@types/node": "^14.0.13",
"@types/strip-ansi": "^5.2.1",
"@types/wrap-ansi": "^3.0.0",
"chai": "^4.2.0",
"chalk": "^2.4.1",
"eslint": "^6.6.0",
"chalk": "^4.1.0",
"eslint": "^7.3.0",
"eslint-config-oclif": "^3.1.0",
"eslint-config-oclif-typescript": "^0.1.0",
"fancy-test": "^1.4.1",
"mocha": "^5.2.0",
"ts-node": "^7.0.1",
"typescript": "3.8.3"
"eslint-config-oclif-typescript": "^0.2.0",
"fancy-test": "^1.4.8",
"mocha": "^7.2.0",
"nock": "^12.0.3",
"ts-node": "^8.10.2",
"typescript": "~3.8.3"
},
"engines": {
"node": ">=8.0.0"
Expand Down
53 changes: 35 additions & 18 deletions src/errors/cli.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,39 @@
// tslint:disable no-implicit-dependencies

import Chalk from 'chalk'
import * as Chalk from 'chalk'
import Clean = require('clean-stack')
import Indent = require('indent-string')
import * as Wrap from 'wrap-ansi'

import {config} from '../config'
import {PrettyPrintableError} from './pretty-print'

export class CLIError extends Error {
oclif: any
/**
* properties specific to internal oclif error handling
*/
export interface OclifError {
oclif: {
exit?: number | false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this is in an oclif property but the other things like ref, code etc are on the actual error object? I always thought the exitCode should be on the error but a personal opinion.

In any case, this is our error object. Some stuff is specific to Salesforce like labels, but having the properties like actions vs. suggestion would be nice.

Copy link
Contributor Author

@chadian chadian Jun 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was more for backwards compatibility with the existing structure of cli error and the oclif property that was defined prior. Then the handle function expects it. It's unlikely people would have a different version of the @oclif/errors or would be creating their own errors with oclif.exit but we didn't want to risk it. Instead we wanted to call out it as an oclif error concern one that isn't involved with the display of the error. With your exitCode are you using a custom handle function in bin/run, too?

};
}

export function addOclifExitCode(error: Record<string, any>, options?: { exit?: number | false }): OclifError {
if (!('oclif' in error)) {
(error as unknown as OclifError).oclif = {}
}

error.oclif.exit = options?.exit === undefined ? 2 : options.exit
return error as OclifError
}

export class CLIError extends Error implements OclifError {
oclif: OclifError['oclif'] = {}

code?: string

constructor(error: string | Error, options: {code?: string; exit?: number | false} = {}) {
const addExitCode = (error: any) => {
error.oclif = error.oclif || {}
error.oclif.exit = options.exit === undefined ? 2 : options.exit
return error
}
if (error instanceof Error) return addExitCode(error as any)
super(error)
addExitCode(this)
constructor(error: string | Error, options: { exit?: number | false } & PrettyPrintableError = {}) {
super(error instanceof Error ? error.message : error)
addOclifExitCode(this, options)
this.code = options.code
}

Expand All @@ -29,6 +42,10 @@ export class CLIError extends Error {
return clean(super.stack!, {pretty: true})
}

/**
* @deprecated `render` Errors display should be handled by display function, like pretty-print
* @return {string} returns a string representing the dispay of the error
*/
render(): string {
if (config.debug) {
return this.stack
Expand All @@ -45,27 +62,27 @@ export class CLIError extends Error {
return output
chadian marked this conversation as resolved.
Show resolved Hide resolved
}

protected get bang() {
get bang() {
let red: typeof Chalk.red = ((s: string) => s) as any
try {
red = require('chalk').red
} catch {}
} catch { }
return red(process.platform === 'win32' ? '»' : '›')
}
}

export namespace CLIError {
export class Warn extends CLIError {
constructor(err: Error | string) {
super(err)
constructor(err: string | Error) {
super(err instanceof Error ? err.message : err)
this.name = 'Warning'
}

protected get bang() {
get bang() {
let yellow: typeof Chalk.yellow = ((s: string) => s) as any
try {
yellow = require('chalk').yellow
} catch {}
} catch { }
return yellow(process.platform === 'win32' ? '»' : '›')
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/errors/exit.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {CLIError} from './cli'
import {CLIError, OclifError} from './cli'

export class ExitError extends CLIError {
export class ExitError extends CLIError implements OclifError {
oclif!: { exit: number }

code = 'EEXIT'
Expand All @@ -10,6 +10,7 @@ export class ExitError extends CLIError {
}

render(): string {
console.warn('`render` methods on CLIError are deprecated in favor of using pretty print functions')
return ''
}
}
69 changes: 69 additions & 0 deletions src/errors/pretty-print.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import * as wrap from 'wrap-ansi'
import indent = require('indent-string')
import * as screen from '../screen'
import {config} from '../config'

export interface PrettyPrintableError {
/**
* messsage to display related to the error
*/
message?: string;

/**
* a unique error code for this error class
*/
code?: string;

/**
* a url to find out more information related to this error
* or fixing the error
*/
ref?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.


/**
* a suggestion that may be useful or provide additional context
*/
suggestion?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

// These exist for backwards compatibility with CLIError
type CLIErrorDisplayOptions = { name?: string; bang?: string }

export function applyPrettyPrintOptions(error: Error, options: PrettyPrintableError): PrettyPrintableError {
const prettyErrorKeys: (keyof PrettyPrintableError)[] = ['message', 'code', 'ref', 'suggestion']

prettyErrorKeys.forEach(key => {
const applyOptionsKey = !(key in error) && options[key]
if (applyOptionsKey) {
(error as PrettyPrintableError)[key] = options[key]
}
})

return error
}

export default function prettyPrint(error: Error & PrettyPrintableError & CLIErrorDisplayOptions) {
if (config.debug) {
return error.stack
}

const {message, code, suggestion, ref, name: errorSuffix, bang} = error

// errorSuffix is pulled from the 'name' property on CLIError
// and is like either Error or Warning
const formattedHeader = message ? `${errorSuffix || 'Error'}: ${message}` : undefined
const formattedCode = code ? `Code: ${code}` : undefined
const formattedSuggestion = suggestion ? `Suggestion: ${suggestion}` : undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Salesforce CLI says "Try this:". I like that a little more than "Suggestion:" but I don't care all that much. It might be worth it to have doc weigh-in. Or I guess to say it another way, if the Salesforce CLI would ever want to use this standard oclif error format, we would want to run it by doc.

const formattedReference = ref ? `Reference: ${ref}` : undefined

const formatted = [formattedHeader, formattedCode, formattedSuggestion, formattedReference]
.filter(Boolean)
.join('\n')

let output = wrap(formatted, screen.errtermwidth - 6, {trim: false, hard: true} as any)
output = indent(output, 3)
output = indent(output, 1, {indent: bang || '', includeEmptyLines: true} as any)
output = indent(output, 1)

return output
}
30 changes: 20 additions & 10 deletions src/handle.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
/* eslint-disable no-process-exit */
/* eslint-disable unicorn/no-process-exit */

import clean = require('clean-stack')

import {config} from './config'
import prettyPrint, {PrettyPrintableError} from './errors/pretty-print'
import {ExitError} from '.'
import clean = require('clean-stack')
import {OclifError, CLIError} from './errors/cli'

export const handle = (err: any) => {
export const handle = (err: Error & Partial<PrettyPrintableError> & Partial<OclifError>) => {
try {
if (!err) err = new Error('no error?')
if (!err) err = new CLIError('no error?')
if (err.message === 'SIGINT') process.exit(1)

const shouldPrint = !(err instanceof ExitError)
const pretty = prettyPrint(err)
const stack = clean(err.stack || '', {pretty: true})
let message = stack
if (err.oclif && typeof err.render === 'function') message = err.render()
if (message) console.error(message)
const exitCode = (err.oclif && err.oclif.exit !== undefined) ? err.oclif.exit : 1

if (shouldPrint) {
console.error(pretty ? pretty : stack)
}

const exitCode = err.oclif?.exit !== undefined && err.oclif?.exit !== false ? err.oclif?.exit : 1

if (config.errorLogger && err.code !== 'EEXIT') {
config.errorLogger.log(stack)
if (stack) {
config.errorLogger.log(stack)
}

config.errorLogger.flush()
.then(() => process.exit(exitCode))
.catch(console.error)
Expand Down
45 changes: 35 additions & 10 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,50 @@ export {Logger} from './logger'
export {config} from './config'

import {config} from './config'
import {CLIError} from './errors/cli'
import {CLIError, addOclifExitCode, OclifError} from './errors/cli'
import {ExitError} from './errors/exit'
import prettyPrint, {PrettyPrintableError, applyPrettyPrintOptions} from './errors/pretty-print'

export {prettyPrint}

export function exit(code = 0): never {
throw new ExitError(code)
}

export function error(input: string | Error, options: {code?: string; exit: false}): void
export function error(input: string | Error, options?: {code?: string; exit?: number}): never
export function error(input: string | Error, options: {code?: string; exit?: number | false} = {}) {
const err = new CLIError(input, options)
export function error(input: string | Error, options: {exit: false} & PrettyPrintableError): void
export function error(input: string | Error, options?: {exit?: number} & PrettyPrintableError): never
export function error(input: string | Error, options: {exit?: number | false} & PrettyPrintableError = {}) {
let err: Error & OclifError

if (typeof input === 'string') {
err = new CLIError(input, options)
} else if (input instanceof Error) {
err = addOclifExitCode(input, options) as Error & OclifError
} else {
throw new TypeError('first argument must be a string or instance of Error')
}

err = applyPrettyPrintOptions(err, options) as Error & OclifError & PrettyPrintableError

if (options.exit === false) {
console.error(err.render ? err.render() : `Error ${err.message}`)
if (config.errorLogger) config.errorLogger.log(err.stack)
const message = prettyPrint(err)
console.error(message)
if (config.errorLogger) config.errorLogger.log(err?.stack ?? '')
} else throw err
}

export function warn(input: string | Error) {
const err = new CLIError.Warn(input)
console.error(err.render ? err.render() : `Warning: ${err.message}`)
if (config.errorLogger) config.errorLogger.log(err.stack)
let err: Error & OclifError

if (typeof input === 'string') {
err = new CLIError.Warn(input)
} else if (input instanceof Error) {
err = addOclifExitCode(input) as Error & OclifError
} else {
throw new TypeError('first argument must be a string or instance of Error')
}

const message = prettyPrint(err)
console.error(message)
if (config.errorLogger) config.errorLogger.log(err?.stack ?? '')
}
Loading