feat!: safe, consistent --json error documents

README.md

@@ -52,6 +52,37 @@ const heroku = new APIClient(config)
 const {body: resources} = await heroku.get('/apps')
 ```
 
+### JSON output
+
+Commands with `static enableJsonFlag = true` print JSON under `--json` or `HEROKU_CONTENT_TYPE=json`. On success, stdout holds what `run()` returned. On failure, stdout holds an error document, the same shape for every command (the `JsonErrorDocument` type):
+
+```json
+{"error": {"message": "Couldn't find that app.", "name": "NotFoundError", "exitCode": 1, "statusCode": 404, "id": "not_found", "resource": "app", "requestId": "…"}}
+```
+
+- The exit code decides success or failure.
+- To tell errors apart, match on `exitCode`, `statusCode`, `id` and `code`. `name` and `message` are for people: the same API failure can have a different `name` depending on the client that made the request.
+- Fields are optional except `message`, `name` and `exitCode`. New fields may be added, so ignore fields you don't know.
+- The document is built from a fixed list of fields, with the API token and credentials redacted. The length limits and redaction rules aren't part of the contract.
+- For a failed API request, `message` is the API's own message, such as `Couldn't find that app.`, whichever HTTP client made the request.
+- `this.exit(n)` prints no document; the process exits with `n`.
+- Spinners (`ux.action`) are silent, and `--prompt` is rejected with the code `HEROKU_PROMPT_WITH_JSON`.
+- `HEROKU_DEBUG=1` also writes the full error, redacted, to stderr.
+
+A command whose flag prints raw output instead of JSON lists that flag in `static jsonSuppressedBy`. When the flag is set, `jsonEnabled()` is false and the command prints its raw output, even under `--json` or `HEROKU_CONTENT_TYPE=json`. A value that comes only from the flag's `default` doesn't count, and an error before or during flag parsing is still printed as an error document. To reject the flag together with `--json`, also give it `exclusive: ['json']`.
+
+```typescript
+export default class Create extends Command {
+  static enableJsonFlag = true
+  static jsonSuppressedBy = ['short']
+  static flags = {
+    short: Flags.boolean({char: 'S', description: 'only output token', exclusive: ['json']}),
+  }
+}
+```
+
+With this, `HEROKU_CONTENT_TYPE=json heroku authorizations:create --short` prints only the token.
+
 ## Usage
 
 This package is primarily used as a dependency in other Heroku CLI plugins and commands. It provides the base functionality needed to implement Heroku CLI commands with proper completion support.

package.json

@@ -10,9 +10,9 @@
     "@heroku/heroku-credential-manager": "^0.2.1",
     "@heroku/heroku-fetch": "^0.2.1",
     "@heroku/http-call": "5.5.2",
-    "@heroku/js-blanket": "^1.0.0",
+    "@heroku/js-blanket": "^1.4.0",
     "@heroku/sdk": "^0.11.0",
-    "@oclif/core": "^5.1.2",
+    "@oclif/core": "^5.1.9",
     "@sentry/node": "^10.45.0",
     "ansis": "^4",
     "debug": "^4.4.0",

src/command.ts

@@ -1,26 +1,60 @@
+import type {Input, ParserOutput} from '@oclif/core/interfaces'
+
 import {Command as Base} from '@oclif/core/command'
+import {CLIError} from '@oclif/core/errors'
 import * as Flags from '@oclif/core/flags'
+import {ux} from '@oclif/core/ux'
 
 import {APIClient, type IOptions} from './api-client.js'
+import {
+  debugErrorText,
+  type JsonErrorDocument,
+  toError,
+  toJsonErrorDocument,
+} from './json-error-document.js'
+import {quietSpinners} from './quiet-spinners.js'
+
+/** The `code` of the error for `--prompt` under JSON output. */
+export const PROMPT_WITH_JSON_ERROR_CODE = 'HEROKU_PROMPT_WITH_JSON'
 
 export abstract class Command extends Base {
   /**
    * Base flags that includes the prompt flag by default
    * Subclasses can override this to customize base flags
    */
   static baseFlags: Record<string, any> = {
     prompt: Flags.boolean({
       description: 'interactively prompt for command arguments and flags',
+      // Enforced in init(), before any prompt; this only documents the conflict.
+      exclusive: ['json'],
       helpGroup: 'GLOBAL',
     }),
   }
 
+  /**
+   * Flags on this command that print raw output, such as a flag that prints only
+   * a token. When one is set, JSON output is off, even under `--json` or
+   * `HEROKU_CONTENT_TYPE=json`. Only this command's own declaration counts: the
+   * same flag name on another command has no effect. The parsed flags decide,
+   * so an error before or during parsing is still a JSON document, and a value
+   * that comes from the flag's `default` doesn't count. Type it against the
+   * command's flags so a typo fails to compile:
+   *
+   * @example
+   * static jsonSuppressedBy: Array<keyof typeof MyCommand.flags> = ['short']
+   */
+  static jsonSuppressedBy: readonly string[] = []
+
   /**
    * Set this to false in a command class to disable the --prompt flag for that command
    */
   static promptFlagActive = true
   _heroku!: APIClient
 
+  private parsedFlagMetadata?: Record<string, {setFromDefault?: boolean}>
+  private parsedFlags?: Record<string, unknown>
+  private restoreSpinners?: () => void
+
   /**
    * Helper function to get baseFlags without the prompt flag
    * Use this when you want to remove the prompt flag in a specific command:
@@ -31,7 +65,7 @@
    *   static flags = { ... }
    * }
    */
   static baseFlagsWithoutPrompt(): Record<string, any> {
     // Destructure to remove the prompt flag
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     const {prompt, ...rest} = this.baseFlags
@@ -42,16 +76,50 @@
   get heroku(): APIClient {
     if (this._heroku) return this._heroku
     const options: IOptions = {
-      debug: process.env.HEROKU_DEBUG === '1' || process.env.HEROKU_DEBUG?.toUpperCase() === 'TRUE',
-      debugHeaders: process.env.HEROKU_DEBUG_HEADERS === '1' || process.env.HEROKU_DEBUG_HEADERS?.toUpperCase() === 'TRUE',
+      debug: isEnvTrue('HEROKU_DEBUG'),
+      debugHeaders: isEnvTrue('HEROKU_DEBUG_HEADERS'),
     }
     this._heroku = new APIClient(this.config, options)
     return this._heroku
   }
 
+  /**
+   * Stops a running spinner before oclif renders the error. oclif stops it only
+   * when JSON output is off, and a running spinner buffers stdout, so the
+   * `--json` error document would otherwise never be written.
+   */
+  async catch(thrown: Error & {exitCode?: number}): Promise<any> {
+    // oclif's catch() reads properties of the thrown value, so `throw undefined` would crash it.
+    const err = toError(thrown) as CLIError & {exitCode?: number}
+    try {
+      if (ux.action.running) ux.action.stop(ux.colorize('bold', ux.colorize('red', '!')))
+    } catch {}
+
+    if (this.jsonEnabled()) {
+      // this.exit() and ux.exit() aren't failures to report: the exit code says it all.
+      if (err.code === 'EEXIT') {
+        process.exitCode ??= err.oclif?.exit ?? 0
+        return
+      }
+
+      if (isEnvTrue('HEROKU_DEBUG')) process.stderr.write(`${debugErrorText(err, this.knownSecrets())}\n`)
+    }
+
+    return super.catch(err)
+  }
+
+  async finally(err: Error | undefined): Promise<any> {
+    this.restoreSpinners?.()
+    this.restoreSpinners = undefined
+    return super.finally(err)
+  }
+
   async init(): Promise<void> {
     await super.init()
 
+    // Under JSON output, spinners write nothing and never take over stdout/stderr.
+    if (this.jsonEnabled()) this.restoreSpinners = quietSpinners()
+
     // Preload credentials so this.heroku.auth is set before run() (async credential-manager + sync checks).
     await this.heroku.getAuth()
 
@@ -64,6 +132,14 @@
       return
     }
 
+    // Prompts wait for answers a script will never type. Checked here because prompting runs
+    // before parse() enforces `exclusive`, and HEROKU_CONTENT_TYPE=json sets no flag at all.
+    if (this.jsonEnabled()) {
+      throw new CLIError('--prompt can\'t be used with --json. Pass the arguments and flags on the command line.', {
+        code: PROMPT_WITH_JSON_ERROR_CODE,
+      })
+    }
+
     // If we get here, we need to prompt for inputs
     const commandId = this.id
     if (!commandId) return
@@ -85,4 +161,57 @@
     const Ctor = this.constructor as typeof Command
     return Ctor.promptFlagActive && ('prompt' in Ctor.baseFlags)
   }
+
+  /**
+   * Whether JSON output is on: oclif's rule (`--json` or `HEROKU_CONTENT_TYPE=json`),
+   * unless a flag in {@link Command.jsonSuppressedBy} is set.
+   */
+  public jsonEnabled(): boolean {
+    return super.jsonEnabled() && !this.jsonSuppressed()
+  }
+
+  /** The current API token and HEROKU_API_KEY, redacted wherever they appear in error output. */
+  protected knownSecrets(): Array<string | undefined> {
+    return [this._heroku?.auth, process.env.HEROKU_API_KEY]
+  }
+
+  protected async parse<F extends Record<string, any>, B extends Record<string, any>, A extends Record<string, any>>(
+    options?: Input<F, B, A>,
+    argv?: string[],
+  ): Promise<ParserOutput<F, B, A>> {
+    const output = await super.parse<F, B, A>(options, argv)
+    this.parsedFlags = output.flags
+    this.parsedFlagMetadata = output.metadata.flags
+    return output
+  }
+
+  /**
+   * The `--json` error document, the same for every command: an allowlisted set
+   * of fields with secrets redacted. See {@link JsonErrorDocument}.
+   */
+  public toErrorJson(err: unknown): {error: JsonErrorDocument} {
+    const fromError = (err as undefined | {exitCode?: unknown})?.exitCode
+    // Report the code the process exits with: oclif's catch() sets process.exitCode
+    // from the same sources before rendering the document.
+    const exitCode = Number(process.exitCode ?? (typeof fromError === 'number' ? fromError : 1))
+    return {error: toJsonErrorDocument(err, {exitCode, secrets: this.knownSecrets()})}
+  }
+
+  /**
+   * Whether a flag in {@link Command.jsonSuppressedBy} is set on the command line
+   * or by its environment variable. Always false before parsing.
+   */
+  private jsonSuppressed(): boolean {
+    const {jsonSuppressedBy} = this.constructor as typeof Command
+    return jsonSuppressedBy.some(name => {
+      const value = this.parsedFlags?.[name]
+      return value !== undefined && value !== false && !this.parsedFlagMetadata?.[name]?.setFromDefault
+    })
+  }
+}
+
+/** Whether the environment variable is `1` or `true`, in any case. */
+function isEnvTrue(name: string): boolean {
+  const value = process.env[name]?.toLowerCase()
+  return value === '1' || value === 'true'
 }

src/credential-error-reporter.ts

@@ -2,6 +2,7 @@ import type {CredentialErrorContext} from '@heroku/heroku-credential-manager'
 import type {ErrorEvent} from '@sentry/node'
 
 import {
+  CREDENTIAL_PATTERNS,
   GDPR_FIELDS,
   HEROKU_FIELDS,
   PCI_FIELDS,
@@ -18,7 +19,9 @@ const DSN =
 
 const scrubber = new Scrubber({
   fields: [...HEROKU_FIELDS, ...GDPR_FIELDS, ...PCI_FIELDS],
-  patterns: [...PII_PATTERNS],
+  // Credentials first, so a URL password is replaced as a whole rather than
+  // partly matched by the email pattern.
+  patterns: [...CREDENTIAL_PATTERNS, ...PII_PATTERNS],
 })
 
 /** Indirection so tests can `sinon.stub` without stubbing the ESM `@sentry/node` namespace. */

src/index.ts

@@ -3,7 +3,7 @@ import {setCredentialErrorReporter} from '@heroku/heroku-credential-manager'
 import {reportCredentialStoreError} from './credential-error-reporter.js'
 
 export * from './api-client.js'
-export {Command, Command as default} from './command.js'
+export {Command, Command as default, PROMPT_WITH_JSON_ERROR_CODE} from './command.js'
 export type * from './completion-types.js'
 // Public completion surface: the catalog the host CLI wires by id, plus the two
 // cache-key helpers. The individual Completion singletons and internal helpers
@@ -20,6 +20,7 @@ export type {AuthEntry} from './credential-manager.js'
 export * from './env-auth.js'
 export * as flags from './flags/index.js'
 export * from './git.js'
+export type {JsonErrorDocument} from './json-error-document.js'
 export {NONINTERACTIVE_LOGIN_ERROR_CODE} from './login.js'
 export * from './mutex.js'
 export * from './particleboard-client.js'