Draft
Conversation
Contributor
Coverage report
Test suite run success3832 tests passing in 1464 suites. Report generated by 🧪jest coverage report action from 127b412 |
442d922 to
127b412
Compare
Contributor
Differences in type declarationsWe 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:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/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
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
"extensions": {...}) instead of formatted error messages/Users/mitch/world/trees/root/src/areas/core/shopify/components/...) that obscure meaningful code locationsApps::Operations::StaticAssetPipeline.perform) making stack traces verboseWHAT 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:
graphql.tserrorHandlerto build structured error messages with custom sectionsgraphql.test.tsKey design decisions:
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.
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 bothdev.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.Shorten Ruby method names by dropping namespace: Full names like
Apps::Operations::StaticAssetPipeline.performare verbose and redundant with the file path. Keeping justStaticAssetPipeline.performreduces visual noise without losing meaning. The file path already shows context.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.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.
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:
All existing tests should continue passing. The new test file covers:
Manual (simulating real errors):
Testing 5xx errors with rich metadata:
packages/cli-kit/src/private/node/api/graphql.ts, temporarily modifyerrorHandlerto inject a test error:shopify app info)PublicMessageError at components/apps/framework/app/services/pipeline.rb:37Testing 4xx client errors:
shopify app deploywith invalid config)Testing environment-specific behavior:
SHOPIFY_CLOUD_ENVIRONMENT=production npm run [command]Post-release steps
n/a
Measuring impact
How do we know this change was effective? Please choose one:
Checklist