Skip to content

Add slice metadata to error reporting #6102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Add slice metadata to error reporting #6102

wants to merge 5 commits into from

Conversation

amcaplan
Copy link
Contributor

Summary

This PR adds slice metadata to error reporting for better error attribution and ownership tracking.

Changes:

  • Add slice_name and slice_id fields to error metadata
  • Implement command ID-based slice detection (e.g., app:dev → app slice, theme:push → theme slice)
  • Store current slice information in global context during command initialization
  • Add comprehensive tests for slice detection logic

Implementation Details:

The slice detection uses a simple and reliable approach based on command ID prefixes:

  • app:* and webhook:* commands → app slice (S-9988b6)
  • theme:* commands → theme slice (S-2d23f6)
  • hydrogen:* commands → hydrogen slice (S-156228)
  • store:* commands → bulk data slice (S-1bc8f5)
  • All other commands default to CLI slice (S-f3a87a)

Test plan

  • Added comprehensive unit tests for slice detection logic
  • All existing tests continue to pass
  • Manually verified error metadata structure

The tests cover all command types and verify the correct slice information is set in the global context.

🤖 Generated with Claude Code

@amcaplan amcaplan requested a review from a team as a code owner July 13, 2025 19:55
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

- Add slice_name and slice_id fields to error metadata for better error attribution
- Implement command ID-based slice detection (app:dev → app slice, theme:push → theme slice, etc.)
- Store current slice in global context during command initialization
- Add comprehensive tests for slice detection logic
- Remove monorepo_zone field as CLI is not in the Shopify monorepo

This enables proper error attribution and ownership in error analytics.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

github-actions bot commented Jul 13, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
78.37% (-0.03% 🔻)
13046/16647
🟡 Branches
72.63% (+0.07% 🔼)
6357/8752
🟡 Functions
78.23% (-0.05% 🔻)
3375/4314
🟡 Lines
78.79% (-0.02% 🔻)
12356/15682
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / shop_details.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴
... / index.ts
0% (-6.67% 🔻)
0% 0%
0% (-7.14% 🔻)
🔴
... / current_user_account.ts
0% (-100% 🔻)
100% 100%
0% (-100% 🔻)
🟢
... / copy.ts
97.06% (-0.08% 🔻)
94.29% 100%
97.06% (-0.08% 🔻)
🔴
... / mock-api-client.ts
3.7% (-0.64% 🔻)
0%
9.09% (-0.91% 🔻)
4% (-0.55% 🔻)
🟢
... / store-import.ts
97.78% (-0.09% 🔻)
85.71% (+5.71% 🔼)
100%
97.73% (-0.1% 🔻)

Test suite run success

3044 tests passing in 1319 suites.

Report generated by 🧪jest coverage report action from 46531ea

amcaplan and others added 3 commits July 13, 2025 23:17
- Fix JSDoc syntax errors by using interface instead of inline type notation
- Remove unnecessary try-catch block in slice detection to satisfy no-catch-all rule
- Remove unused outputDebug import
- Ensure proper TypeScript type definitions for SliceInfo

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Remove complex command ID parsing logic from base-command.ts
- Use existing cmd_all_plugin metadata field for slice identification
- Remove global context slice tracking as it's no longer needed
- Implement getSliceNameAndId() function based on plugin name patterns
- Remove all slice detection tests as they're no longer applicable
- Match implementation pattern from prototype-slices branch

This approach is more reliable as it uses metadata that's already being
collected by the analytics system, rather than trying to parse command IDs.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Implement slice identification for better error attribution:
- Add getSliceNameAndId function to map plugins to slices
- Include slice_name and slice_id in Bugsnag metadata
- Use simpler plugin-based approach from PR #6093
- Add comprehensive test coverage with DRY test structure
- Fix sendErrorToBugsnag to return undefined for unhandled when not reporting

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
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/public/node/custom-oclif-loader.d.ts
@@ -1,6 +1,5 @@
-import { Command, Config } from '@oclif/core';
-import { Options } from '@oclif/core/interfaces';
+import { Command, Config, Interfaces } from '@oclif/core';
 export declare class ShopifyConfig extends Config {
-    constructor(options: Options);
+    constructor(options: Interfaces.Options);
     customPriority(commands: Command.Loadable[]): Command.Loadable | undefined;
 }
\ No newline at end of file
packages/cli-kit/dist/public/node/error-handler.d.ts
@@ -36,4 +36,8 @@ export declare function cleanStackFrameFilePath({ currentFilePath, projectRoot,
  *
  */
 export declare function registerCleanBugsnagErrorsFromWithinPlugins(config: Interfaces.Config): Promise<void>;
-export declare function addBugsnagMetadata(event: any, config: Interfaces.Config): Promise<void>;
\ No newline at end of file
+export declare function addBugsnagMetadata(event: any, config: Interfaces.Config): Promise<void>;
+export declare function getSliceNameAndId(plugin: string): {
+    slice_name: string;
+    slice_id: string;
+};
\ No newline at end of file

@amcaplan
Copy link
Contributor Author

Closing as slices are not going to be supported for this repo.

@amcaplan amcaplan closed this Jul 21, 2025
@amcaplan amcaplan deleted the add-slices branch July 21, 2025 10:09
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