Skip to content

Don't send user hydrogenStorefronts access denial error to observe#3654

Merged
kdaviduik merged 2 commits intomainfrom
04-01-add_error_handling_for_hydrogenstorefronts_access_denial
Apr 1, 2026
Merged

Don't send user hydrogenStorefronts access denial error to observe#3654
kdaviduik merged 2 commits intomainfrom
04-01-add_error_handling_for_hydrogenstorefronts_access_denial

Conversation

@lucyxiang
Copy link
Copy Markdown
Contributor

@lucyxiang lucyxiang commented Apr 1, 2026

WHY are these changes introduced?

Don't send user GraphQL permission (access denied) errors to observe (~1k events/ last 30 days). Observe should only have bug/ actionable errors

https://observe.shopify.io/a/observe/errors/12590085769547859575?r={"from"%3A"now-30d"%2C"to"%3A"now"}&tz=browser&f={"resource.service.name"%3A["cli"]%2C"slice_name"%3A["hydrogen"]}

WHAT is this pull request doing?

HOW to test your changes?

Post-merge steps

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

Copy link
Copy Markdown
Contributor Author

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

@shopify
Copy link
Copy Markdown
Contributor

shopify bot commented Apr 1, 2026

Oxygen deployed a preview of your 04-01-add_error_handling_for_hydrogenstorefronts_access_denial branch. Details:

Storefront Status Preview link Deployment details Last update (UTC)
Skeleton (skeleton.hydrogen.shop) ✅ Successful (Logs) Preview deployment Inspect deployment April 1, 2026 8:22 PM

Learn more about Hydrogen's GitHub integration.

@lucyxiang lucyxiang marked this pull request as ready for review April 1, 2026 15:57
@lucyxiang lucyxiang requested a review from a team as a code owner April 1, 2026 15:57
@github-actions

This comment has been minimized.

@lucyxiang lucyxiang changed the title Add error handling for hydrogenStorefronts access denial Don't send hydrogenStorefronts access denial error to observe Apr 1, 2026
@lucyxiang lucyxiang changed the title Don't send hydrogenStorefronts access denial error to observe Don't send user hydrogenStorefronts access denial error to observe Apr 1, 2026
Copy link
Copy Markdown
Contributor

@fredericoo fredericoo left a comment

Choose a reason for hiding this comment

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

needs changeset

can i ask you to elaborate on what this does or what issue it fixes? ◡̈

@lucyxiang
Copy link
Copy Markdown
Contributor Author

lucyxiang commented Apr 1, 2026

Does it need a changeset? IMO this change is not user-facing or noteworthy changes as this PR stops us from sending user related GraphQL permission (access denied) errors to observe. I updated the PR description

@lucyxiang lucyxiang requested a review from fredericoo April 1, 2026 17:19
@lucyxiang lucyxiang force-pushed the 04-01-add_error_handling_for_hydrogenstorefronts_access_denial branch from ba993f7 to e5b299d Compare April 1, 2026 18:23
Copy link
Copy Markdown
Contributor

@kdaviduik kdaviduik left a comment

Choose a reason for hiding this comment

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

PR LGTM but it needs a changeset because changesets are what trigger a new release. Without a changeset we would just be fixing it in our repo but it never would be able to release

@lucyxiang lucyxiang force-pushed the 04-01-add_error_handling_for_hydrogenstorefronts_access_denial branch from e5b299d to fb01d94 Compare April 1, 2026 19:00
@lucyxiang lucyxiang force-pushed the 04-01-add_error_handling_for_hydrogenstorefronts_access_denial branch from fb01d94 to afa2a4c Compare April 1, 2026 19:14
Copy link
Copy Markdown
Collaborator

@andguy95 andguy95 left a comment

Choose a reason for hiding this comment

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

LGTM!

@kdaviduik kdaviduik dismissed fredericoo’s stale review April 1, 2026 21:28

requested changes resolved

@kdaviduik kdaviduik merged commit c4aa7e3 into main Apr 1, 2026
20 of 21 checks passed
@kdaviduik kdaviduik deleted the 04-01-add_error_handling_for_hydrogenstorefronts_access_denial branch April 1, 2026 21:29
itsjustriley added a commit that referenced this pull request Apr 9, 2026
Adds the changelog entry for the 2026.1.4 release to enable `h2 upgrade`
CLI detection. Key changes in this release:
- Storefront MCP proxy for AI agent integration (PR #3572)
- useCustomAuthDomain opt-out for custom HTTPS/ngrok setups (PR #3642)
- GraphQL access denied errors handled as user errors (PR #3654)
- vite bumped to ^6.4.2

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
itsjustriley added a commit that referenced this pull request Apr 9, 2026
* chore: update changelog.json for 2026.1.4

Adds the changelog entry for the 2026.1.4 release to enable `h2 upgrade`
CLI detection. Key changes in this release:
- Storefront MCP proxy for AI agent integration (PR #3572)
- useCustomAuthDomain opt-out for custom HTTPS/ngrok setups (PR #3642)
- GraphQL access denied errors handled as user errors (PR #3654)
- vite bumped to ^6.4.2

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: improve changelog.json entries for 2026.1.4

The original entries had several merchant-clarity issues caught in review:

- MCP proxy entry lacked actionable setup guidance (no endpoint URL, no
  docs link). Added /api/mcp path example and link to
  shopify.dev/docs/apps/build/storefront-mcp/servers/storefront.
- Same entry conflated the new MCP feature with the unrelated GraphQL
  route file cleanup. The route deletion stems from proxyStandardRoutes
  (December 2025), not MCP. Separated with "Separately..." sentence.
- useCustomAuthDomain entry now leads with the merchant's situation
  ("If you use a custom HTTPS domain...") for easier scanning.
- GraphQL access denied fix reworded from internal monitoring framing
  to merchant-facing language.
- Added missing fix entry for PR #3672 (@xstate/fsm bundling regression
  from 2026.1.3 — vite.config.ts change had no changeset requirement
  but fix is user-visible in strict pnpm setups).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: trim changelog.json entries to match established verbosity pattern

Previous changelog entries use 1-2 concise sentences. The MCP proxy
and useMachine entries were substantially longer. Trimmed both:

- MCP proxy: dropped docs link, dev URL example, and route cleanup note
  (the proxy conflation concern is resolved by simply not mentioning
  GraphQL at all)
- useMachine fix: dropped Vite implementation details; the merchant
  action is clear from a single sentence

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

4 participants