Skip to content

Handle expanded core errors#6852

Draft
MitchLillie wants to merge 2 commits intomainfrom
hd39/hosted-app-dx
Draft

Handle expanded core errors#6852
MitchLillie wants to merge 2 commits intomainfrom
hd39/hosted-app-dx

Conversation

@MitchLillie
Copy link
Contributor

@MitchLillie MitchLillie commented Feb 11, 2026

WHY are these changes introduced?

When GraphQL errors occur in CLI commands, the error output was dumping raw JSON with nested extensions, app_stacktrace, and deeply nested deployment paths, making it hard for developers to understand what went wrong. This was especially problematic for 5xx server errors where developers needed quick access to request IDs and relevant stack traces.

Problems solved:

  1. Raw JSON dumps ("extensions": {...}) instead of formatted error messages
  2. Absolute deployment paths (/Users/mitch/world/trees/root/src/areas/core/shopify/components/...) that obscure meaningful code locations
  3. No direct links to Observe logs (in production) or local search commands (in dev)
  4. Fully-qualified Ruby namespaces (Apps::Operations::StaticAssetPipeline.perform) making stack traces verbose
  5. 4xx and 5xx errors handled identically, despite different debugging needs

WHAT is this pull request doing?

Enhances GraphQL error handling to provide clean, actionable error messages with structured diagnostic information. Server errors (5xx) now display custom sections with request IDs (linked to Observe in production), exception details, and formatted stack traces. Client errors (4xx/200) show clean bullet-pointed messages without raw JSON.

Changes:

File Change
graphql.ts Added error extraction and formatting functions; refactored errorHandler to build structured error messages with custom sections
graphql.test.ts Added 600 lines of comprehensive test coverage for error parsing, path stripping, stack trace formatting, and URL generation

Key design decisions:

  1. Server errors get custom sections; client errors get clean text: Server errors (5xx) are unexpected and need debugging context — developers need request IDs, exception classes, and stack traces. Client errors (4xx/200 with errors) are validation/input issues — developers just need the error message. Custom sections add visual weight appropriate for server failures.

  2. Strip paths at "components/" boundary first: Shopify's monorepo has a stable structural marker (components/) that's consistent across all environments. Stripping to this boundary works in both dev.shopify.io (paths like /Users/mitch/world/trees/root/src/areas/core/shopify/components/...) and production deployments (/deploy/app/...). Falls back to common-prefix stripping for non-monorepo paths.

  3. Shorten Ruby method names by dropping namespace: Full names like Apps::Operations::StaticAssetPipeline.perform are verbose and redundant with the file path. Keeping just StaticAssetPipeline.perform reduces visual noise without losing meaning. The file path already shows context.

  4. Production gets Observe links; local gets grep commands: In production, logs are centralized in Observe — direct links save time. Locally, logs are in terminal output or ~/world/trees/root/log — grep commands are more useful than broken links. Environment detection makes the right choice automatic.

  5. Extract and dedupe error messages separately: GraphQL can return duplicate messages or messages with empty strings. Extracting messages into a clean array (with deduplication) before formatting produces consistent output and avoids "• " bullet points with empty content.

  6. Truncate stack traces at 8 entries: Full 20+ line stack traces bury the useful information. Showing 8 entries with a "... N more" indicator surfaces the relevant frames (usually top 3-5) without overwhelming the terminal. Developers can still access full stacks in Observe or logs.

How to test your changes?

Automated:

npx vitest run packages/cli-kit/src/private/node/api/graphql.test.ts

All existing tests should continue passing. The new test file covers:

  • Error message extraction and deduplication
  • Metadata extraction (request IDs, exception classes, stack traces)
  • Path stripping (components boundary and common-prefix fallback)
  • Ruby stack entry parsing
  • Error handler behavior for 4xx, 5xx, and 200-with-errors cases
  • Observe URL generation based on environment

Manual (simulating real errors):

Testing 5xx errors with rich metadata:

  1. In packages/cli-kit/src/private/node/api/graphql.ts, temporarily modify errorHandler to inject a test error:
    export function errorHandler(api: string): (error: unknown, requestId?: string) => unknown {
      return (error: unknown, requestId?: string) => {
        // Inject test error
        const testError = new ClientError(
          {
            status: 500,
            headers: new Map(),
            errors: [{
              message: 'uncaught throw #<StandardError: Not implemented>',
              extensions: {
                request_id: 'test-request-id-12345',
                exception_class: 'PublicMessageError',
                source: {
                  file: '/Users/mitch/world/trees/root/src/areas/core/shopify/components/apps/framework/app/services/pipeline.rb',
                  line: 37,
                },
                app_stacktrace: [
                  "/Users/mitch/world/trees/root/src/areas/core/shopify/components/apps/framework/app/services/pipeline.rb:37:in 'Apps::Operations::StaticAssetPipeline.perform'",
                  "/Users/mitch/world/trees/root/src/areas/core/shopify/components/apps/app/plugins/handler.rb:20:in 'Handler#process'",
                ],
              },
            }],
            data: undefined,
          },
          { query: 'test query' }
        )
        // ... rest of existing code
  2. Run any CLI command that makes GraphQL requests (e.g., shopify app info)
  3. Verify error output shows:
    • Clean headline: "The [API Name] GraphQL API returned an internal server error (500)."
    • Bold error message (no raw JSON)
    • Request ID section with link (production) or grep command (local)
    • Exception section: PublicMessageError at components/apps/framework/app/services/pipeline.rb:37
    • Stack trace section with shortened method names and stripped paths
  4. Remove test injection

Testing 4xx client errors:

  1. Trigger a validation error (e.g., shopify app deploy with invalid config)
  2. Verify error shows clean bullet points without raw JSON

Testing environment-specific behavior:

  1. Run with SHOPIFY_CLOUD_ENVIRONMENT=production npm run [command]
  2. Verify Request ID section shows Observe link
  3. Run without the env var
  4. Verify Request ID section shows grep command

Post-release steps

n/a

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2026

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 79.03% 14686/18583
🟡 Branches 73.38% 7327/9985
🟡 Functions 79.19% 3729/4709
🟡 Lines 79.38% 13876/17480

Test suite run success

3832 tests passing in 1464 suites.

Report generated by 🧪jest coverage report action from 127b412

@github-actions
Copy link
Contributor

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/private/node/api/graphql.d.ts
@@ -3,4 +3,57 @@ export declare function debugLogRequestInfo(api: string, query: string, url: str
     [key: string]: string;
 }): void;
 export declare function sanitizeVariables(variables: Variables): string;
-export declare function errorHandler(api: string): (error: unknown, requestId?: string) => unknown;
\ No newline at end of file
+interface ParsedStackEntry {
+    method: string;
+    file: string;
+    line: string | undefined;
+}
+interface GraphQLErrorMeta {
+    requestId: string | undefined;
+    exceptionClass: string | undefined;
+    sourceFile: string | undefined;
+    sourceLine: number | undefined;
+    stackTrace: ParsedStackEntry[];
+}
+/**
+ * Extracts human-readable error messages from a GraphQL error response.
+ * Filters out empty strings and deduplicates messages.
+ */
+export declare function extractErrorMessages(errors: unknown): string[];
+/**
+ * Extracts structured metadata from all GraphQL error extensions.
+ * Pulls request ID, exception class, source location, and the server stack trace.
+ */
+export declare function extractGraphQLErrorMeta(errors: unknown): GraphQLErrorMeta;
+/**
+ * Parses a Ruby stack trace entry into its component parts.
+ * Handles formats like: "/path/to/file.rb:37:in 'Module::Class#method'"
+ * Preserves the full file path for detailed debugging.
+ */
+export declare function parseRubyStackEntry(entry: string): ParsedStackEntry;
+/**
+ * Strips deployment and organizational prefixes from a server-side file path,
+ * leaving just the meaningful code location starting from a known structural boundary.
+ *
+ * In Shopify's monorepo, code lives under "components/" — everything before that
+ * is deployment root + area/team organization that varies between environments.
+ * Returns the path unchanged if no structural marker is found.
+ */
+export declare function stripDeploymentPrefix(filePath: string): string;
+/**
+ * Finds the longest common directory prefix across multiple file paths.
+ * Used as a fallback when structural markers aren't found — strips deployment-specific
+ * root directories from server-side paths for cleaner display.
+ *
+ * Requires at least 2 non-empty paths to compute a prefix; otherwise returns "".
+ * Returns the prefix including a trailing "/" ready for stripping.
+ */
+export declare function findCommonPathPrefix(paths: string[]): string;
+/**
+ * Builds the Observe logs URL for a request ID.
+ * Returns the production Observe URL. For local development, returns undefined since logs
+ * are not sent to Observe and should be viewed in the terminal output instead.
+ */
+export declare function buildObserveUrl(requestId: string): string | undefined;
+export declare function errorHandler(api: string): (error: unknown, requestId?: string) => unknown;
+export {};
\ No newline at end of file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant