Skip to content

Handle TOML parse failures as user errors#7704

Merged
isaacroldan merged 1 commit into
mainfrom
06-03-handle_toml_parse_failures_as_user_errors
Jun 3, 2026
Merged

Handle TOML parse failures as user errors#7704
isaacroldan merged 1 commit into
mainfrom
06-03-handle_toml_parse_failures_as_user_errors

Conversation

@isaacroldan
Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan commented Jun 3, 2026

Why

Malformed TOML config files were reported as unexpected CLI crashes.

Fixes https://github.com/shop/issues/issues/47669

What

Classify TomlFileError as an expected user error and add regression coverage for TOML/environment parsing.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Jun 3, 2026
@isaacroldan isaacroldan marked this pull request as ready for review June 3, 2026 10:09
@isaacroldan isaacroldan requested a review from a team as a code owner June 3, 2026 10:09
Copilot AI review requested due to automatic review settings June 3, 2026 10:09
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/toml/toml-file.d.ts
 import { JsonMapType } from './codec.js';
+import { AbortError } from '../error.js';
 /**
  * An error on a TOML file — missing or malformed.
- * Extends Error so it can be thrown. Carries path and a clean message suitable for JSON output.
+ * Carries path and a clean message suitable for JSON output.
  */
-export declare class TomlFileError extends Error {
+export declare class TomlFileError extends AbortError {
     readonly path: string;
     constructor(path: string, message: string);
 }
 /**
  * General-purpose TOML file abstraction.
  *
  * Provides a unified interface for reading, patching, removing keys from, and replacing
  * the content of TOML files on disk.
  *
  * - `read` populates content from disk
  * - `patch` does surgical WASM-based edits (preserves comments and formatting)
  * - `remove` deletes a key by dotted path (preserves comments and formatting)
  * - `replace` does a full re-serialization (comments and formatting are NOT preserved).
  * - `transformRaw` applies a function to the raw TOML string on disk.
  */
 export declare class TomlFile {
     /**
      * Read and parse a TOML file from disk. Throws {@link TomlFileError} if the file
      * doesn't exist or contains invalid TOML.
      *
      * @param path - Absolute path to the TOML file.
      * @returns A TomlFile instance with parsed content.
      */
     static read(path: string): Promise<TomlFile>;
     readonly path: string;
     content: JsonMapType;
     readonly errors: TomlFileError[];
     constructor(path: string, content: JsonMapType);
     /**
      * Surgically patch values in the TOML file, preserving comments and formatting.
      *
      * Accepts a nested object whose leaf values are set in the TOML. Intermediate tables are
      * created automatically. Setting a leaf to `undefined` removes it (use `remove()` for a
      * clearer API when deleting keys).
      *
      * @example
      * ```ts
      * await file.patch({build: {dev_store_url: 'my-store.myshopify.com'}})
      * await file.patch({application_url: 'https://example.com', auth: {redirect_urls: ['...']}})
      * ```
      */
     patch(changes: {
         [key: string]: unknown;
     }): Promise<void>;
     /**
      * Remove a key from the TOML file by dotted path, preserving comments and formatting.
      *
      * @param keyPath - Dotted key path to remove (e.g. 'build.include_config_on_deploy').
      * @example
      * ```ts
      * await file.remove('build.include_config_on_deploy')
      * ```
      */
     remove(keyPath: string): Promise<void>;
     /**
      * Replace the entire file content. The file is fully re-serialized — comments and formatting
      * are NOT preserved.
      *
      * @param content - The new content to write.
      * @example
      * ```ts
      * await file.replace({client_id: 'abc', name: 'My App'})
      * ```
      */
     replace(content: JsonMapType): Promise<void>;
     /**
      * Transform the raw TOML string on disk. Reads the file, applies the transform function
      * to the raw text, writes back, and re-parses to keep `content` in sync.
      *
      * Use this for text-level operations that can't be expressed as structured edits —
      * e.g. Injecting comments or positional insertion of keys in arrays-of-tables.
      * Subsequent `patch()` calls will preserve any comments added this way.
      *
      * @param transform - A function that receives the raw TOML string and returns the modified string.
      * @example
      * ```ts
      * await file.transformRaw((raw) => `# Header comment\n${raw}`)
      * ```
      */
     transformRaw(transform: (raw: string) => string): Promise<void>;
     private decode;
 }

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the TOML file error type so TOML parse failures (and missing file errors) are treated as expected user errors by the CLI error-classification logic, rather than being considered unexpected/bug-reportable.

Changes:

  • Change TomlFileError to extend AbortError so it’s classified as non-unexpected by shouldReportErrorAsUnexpected.
  • Add coverage to assert malformed TOML errors are treated as expected user errors in both the TOML abstraction tests and environment-loading tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/cli-kit/src/public/node/toml/toml-file.ts Makes TomlFileError an AbortError so TOML parsing/missing-file failures are treated as expected user errors.
packages/cli-kit/src/public/node/toml/toml-file.test.ts Adds a test asserting invalid TOML errors are not reported as unexpected.
packages/cli-kit/src/public/node/environments.test.ts Adds a test asserting malformed environment TOML fails with an expected (non-unexpected) user error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@isaacroldan isaacroldan added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit 9bdf0a4 Jun 3, 2026
30 checks passed
@isaacroldan isaacroldan deleted the 06-03-handle_toml_parse_failures_as_user_errors branch June 3, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants