Group CLI crash reports on structured error signals

[stephanie-shopify]

WHY are these changes introduced?

Fixes #7891. which comes from resiliency issue

A single Bugsnag bucket (groupingHash 16864652937831232783) had become a catch-all for ~1,170 unrelated GraphQL/API errors spanning app/theme/store/hydrogen. That makes the bucket un-routable (five owning teams in one group), mixes expected/transient noise with genuine regressions, and manufactures false P1s.

The earlier fix attempt (#7894) addressed the symptom by regex-reparsing the stringified error message to reconstruct type information, and removed expected-error suppression entirely (reporting every AbortError). This PR is the redesign: it groups on the structured signals the error already carries.

The key insight: in sendErrorToBugsnag, only the reportable copy is flattened to new Error(error.message) — the original typed error is still in scope and already carries statusCode + errors[].extensions.code. So we group on facts, not on a string we just serialized.

WHAT is this pull request doing?

• New error-grouping.ts — derives {httpStatus, code, errorClass} from the original error (GraphQLClientError and raw graphql-request ClientError) and maps them via an explicit decision table:
  • THROTTLED / 429 → rate_limit
  • 401 → authentication
  • 403 / ACCESS_DENIED → permission
  • 5xx → server
  • else → shared keyword categorizer (fallback only)
  • unknown → leave groupingHash unset so Bugsnag's stack-trace grouping applies and distinct unknown bugs aren't merged.
• error-handler.ts — sets event.groupingHash only when a real category resolves, and emits error_grouping metadata (slice_name, http_status, error_code, error_class) so backend dashboards/routing work regardless of CLI version (the hash is client-side; old versions keep the old hash until adoption rolls).
• error.ts — raw ClientError with 401/429/THROTTLED is now treated as expected and kept out of crash reporting.
• The shared error-categorizer.ts / storage.ts (Monorail analytics taxonomy) are untouched — their tests stay green, proving the error:${category}:${signature} events are unaffected.
• A short error-grouping.DESIGN.md records the problem, constraints, and the three decisions.

How to test your changes?

pnpm vitest run packages/cli-kit/src/private/node/analytics/error-grouping.test.ts
pnpm vitest run packages/cli-kit/src/public/node/error-handler.test.ts
pnpm vitest run packages/cli-kit/src/public/node/error.test.ts
pnpm vitest run packages/cli-kit/src/private/node/analytics/   # categorizer + storage stay green

Decision-table matrix (asserted in tests): 403 ACCESS_DENIED → theme:permission:http-403-access-denied; 401 → *:authentication:http-401; THROTTLED → *:rate_limit:* and not reported; 5xx → server; raw ClientError; generic Error → groupingHash unset (stack grouping).

Rollout note

Setting groupingHash re-buckets all CLI errors once — a one-time grouping migration across CLI error dashboards. This is intended and an improvement, but should be flagged to whoever owns the Bugsnag/Observe dashboards before merge. The structured metadata tags give backend routing that works across all CLI versions immediately, which is the mitigation while client adoption rolls.

Follow-ups (out of scope)

• At-source category/code on the FatalError hierarchy + API wrappers (the "proper" version of this).
• Folding status/code grouping into error-categorizer.ts (needs a coordinated storage.ts update + pinned tests).
• Alert on *:unknown:* and cli:* bucket growth so the catch-all surfaces itself.
• Tie Bugsnag severity to SLO/error-budget burn rather than aggregate volume.

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows) — pure TS, no platform-specific paths.
• I've considered possible documentation changes — none needed (internal crash-reporting behavior).
• I've considered analytics changes to measure impact — the shared Monorail taxonomy is deliberately untouched; new Bugsnag error_grouping metadata is additive.
• The change is user-facing — patch bump added via changeset.

🤖 Generated with Claude Code

[mbarak]

🐛 Bug: firstExtensionCode only reads errors[0].extensions.code, but GraphQL responses can include multiple errors and existing code in this package treats those arrays as multi-entry structures. A later error may carry THROTTLED, 429, or ACCESS_DENIED, and App Management errors can carry nested extensions.app_errors.errors[].category === 'access_denied'. Missing those shapes can route known API conditions to a fallback or default bucket instead of the intended structured grouping.

Suggestion: Consider replacing the first-error lookup with a small normalization helper that scans every GraphQL error, recognizes both THROTTLED and 429 as rate-limit signals, recognizes ACCESS_DENIED and nested App Management access_denied as permission signals, and is shared with the reporting-suppression logic where possible.

[stephanie-shopify]

Agreed. Replaced the first-error lookup with a shared scanner that walks every error and recognizes THROTTLED/429 (rate-limit) and ACCESS_DENIED plus the nested App Management extensions.app_errors.errors[].category === 'access_denied' (permission). It lives in a new dependency-free graphql-error-codes.ts so it can be shared with the suppression logic in error.ts — error.ts can't import error-grouping.ts directly without creating an error.ts → headers.ts → error.ts cycle. Grouping picks a routable code (rate-limit, then permission) so a benign leading error can't mask one further down the array.

[dmerand]

Nit: This helper is not really analytics-specific. It encodes GraphQL/API error semantics used for crash-report suppression and routing.

Recommendation: not required for this PR, but if this grows, move it closer to the API layer, e.g.

  packages/cli-kit/src/private/node/api/graphql-error-codes.ts

or

  packages/cli-kit/src/private/node/api/error-codes.ts

That would make the dependency direction easier to understand: API error shape → reporting decisions, rather than analytics → public error handling.

[dmerand]

Nit: This shape being constructed directly in error-handler is fine for now, but this PR explicitly says backend dashboards/routing should rely on these tags. Once external dashboards depend on them, the metadata shape becomes a contract.

Recommendation: follow-up only. Consider a small typed formatter near resolveErrorGrouping, e.g. errorGroupingMetadata(resolution, sliceName), so future changes to hash/category/signal behavior update metadata in one place.

[isaacroldan]

This is not an user-facing change, please remove the changeset 🙏

[stephanie-shopify]

addressed

[isaacroldan]

you don't really need to check for not-undefined here no? assigning an undefined value should be safe

[stephanie-shopify]

It would be safe with the current field-by-field ?? merge, but I’m keeping the guard to preserve this helper’s invariant: it only emits fields that are actually present. That avoids reintroducing “present but undefined hammers fallback” class of bug if this object is later spread/merged elsewhere.

[isaacroldan]

an slugify function already exists at '@shopify/cli-kit/common/string', let's reuse that.

[stephanie-shopify]

addressed

[isaacroldan]

Please add organization to this list, that's a new slice of commands added recently

[stephanie-shopify]

addressed

[github-actions]

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

packages/cli-kit/dist/private/node/analytics/error-grouping.d.ts

/**
 * Structured signals extracted from an error, used to group it into a meaningful Bugsnag bucket.
 *
 * These are read from the *original* typed error (before it is flattened to a generic `Error`
 * for reporting), so we group on facts the error already carries rather than by re-parsing a
 * stringified message.
 */
interface ErrorGroupingSignals {
    httpStatus?: number;
    code?: string;
    errorClass?: string;
}
/**
 * The fully resolved grouping decision for an error: the Bugsnag grouping hash (or `undefined` to
 * fall back to stack-trace grouping), the semantic category, and the structured signals that drove
 * the decision. The reporter uses a single resolution for both `event.groupingHash` and the
 * `error_grouping` metadata so they can never disagree.
 */
interface ResolvedErrorGrouping {
    hash?: string;
    category?: string;
    signals: ErrorGroupingSignals;
}
/**
 * Extracts structured grouping signals from an error object.
 *
 * Only assigns a field when a value is actually present, so the result can be safely merged over
 * message-derived signals without an explicit `undefined` erasing a recovered value.
 *
 * @param error - The original error (any type).
 * @returns The HTTP status, GraphQL error code, and error class name when available.
 */
export declare function errorGroupingSignals(error: unknown): ErrorGroupingSignals;
/**
 * Resolves the full grouping decision for an error: hash, category, and the signals behind it.
 *
 * Categories are resolved structured-first (HTTP status / GraphQL code), falling back to the
 * shared keyword categorizer only for untyped errors. When no meaningful category can be
 * resolved, `hash` is `undefined` so the caller leaves `event.groupingHash` unset and Bugsnag's
 * default stack-trace grouping applies — this avoids merging genuinely distinct unknown bugs.
 *
 * @param error - The original error (any type).
 * @param sliceName - The product slice (`app`, `theme`, `store`, `hydrogen`, or `cli`).
 * @returns The resolved hash (or `undefined`), category, and structured signals.
 */
export declare function resolveErrorGrouping(error: unknown, sliceName: string): ResolvedErrorGrouping;
/**
 * Builds a Bugsnag grouping hash of the form `${slice}:${category}:${signature}`.
 *
 * Thin wrapper over {@link resolveErrorGrouping} for callers that only need the hash.
 *
 * @param error - The original error (any type).
 * @param sliceName - The product slice (`app`, `theme`, `store`, `hydrogen`, or `cli`).
 * @returns The grouping hash, or `undefined` to fall back to stack-trace grouping.
 */
export declare function errorGroupingHash(error: unknown, sliceName: string): string | undefined;
export {};

packages/cli-kit/dist/private/node/analytics/graphql-error-codes.d.ts

/**
 * Pure helpers for reading routing-relevant codes out of a GraphQL error response.
 *
 * This module is intentionally dependency-free: it is imported by both the grouping logic
 * (`error-grouping.ts`) and the crash-report suppression logic (`../../public/node/error.ts`).
 * `error.ts` cannot import `error-grouping.ts` directly — that would create an
 * `error.ts → headers.ts → error.ts` import cycle — so the shared scanning logic lives here,
 * where it imports nothing from cli-kit.
 */
/**
 * Collects every routing-relevant code from a GraphQL `errors` value.
 *
 * Scans *all* errors (not just the first), reading both the top-level `extensions.code` and the
 * nested App Management shape `extensions.app_errors.errors[].category`. The API can also return
 * `errors` as a string (e.g. some 401s), which yields no codes.
 *
 * @param errors - The `errors` value from a GraphQL response (array, string, or undefined).
 * @returns Every string code/category found, in document order.
 */
export declare function graphQLErrorCodes(errors: unknown): string[];
/**
 * Whether a single code is a rate-limit signal (`THROTTLED` or `429`).
 *
 * Mirrors the established shape detected by `errorsIncludeStatus429` in `private/node/api.ts`,
 * where `extensions.code === '429'` signals rate limiting even at HTTP 200.
 */
export declare function isRateLimitCode(code: string | undefined): boolean;
/**
 * Whether a single code/category is a permission / access-denied signal.
 */
export declare function isPermissionCode(code: string | undefined): boolean;
/**
 * Whether any GraphQL error carries a rate-limit code. Convenience for suppression logic that only
 * needs a boolean over the raw `errors` value.
 *
 * @param errors - The `errors` value from a GraphQL response.
 * @returns True when a rate-limit code is present in any error.
 */
export declare function hasRateLimitCode(errors: unknown): boolean;

Existing type declarations

We found no diffs with existing type declarations

[stephanie-shopify]

/merge