feat: show a clear error when trying to use CAAPI untunnelled in localhost by fredericoo · Pull Request #3504 · Shopify/hydrogen

fredericoo · 2026-02-25T15:54:05Z

WHY are these changes introduced?

Local development has a bad failure mode for Customer Account OAuth:

When a developer opens /account on localhost without tunneling, the app redirects into the OAuth flow and eventually fails with a redirect URI error from Shopify. The error is late and unclear, so it does not tell developers what they need to do next.

WHAT is this pull request doing?

throws an error and offers a way out for users:
zero impact in bundle size: because NODE_ENV is static at build time, we can get rid of the code path entirely when bundling with vite

HOW to test your changes?

Install deps and build Hydrogen package:
- pnpm --dir packages/hydrogen build
Run unit tests:
- pnpm --dir packages/hydrogen test -- src/customer/customer.test.ts
Start a dev app without customer-account tunneling and visit:
- http://localhost:3000/account
Verify result:
- You should not be redirected to Shopify OAuth.
- You should see a 400 error page with guidance to use --customer-account-push and a *.tryhydrogen.dev URL.
Start dev with tunneling (--customer-account-push) and open the provided *.tryhydrogen.dev URL.
Verify /account continues into normal Customer Account login flow.

Post-merge steps

None.

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

shopify · 2026-02-25T15:55:52Z

Oxygen deployed a preview of your fb-better-feedback-for-cap branch. Details:

Storefront	Status	Preview link	Deployment details	Last update (UTC)
Skeleton (skeleton.hydrogen.shop)	✅ Successful (Logs)	Preview deployment	Inspect deployment	February 26, 2026 6:51 PM

Learn more about Hydrogen's GitHub integration.

kdaviduik

massive DX improvement ❤️

packages/hydrogen/src/customer/customer.ts

kdaviduik · 2026-02-26T05:07:31Z

packages/hydrogen/src/customer/customer.test.ts

+      it('Throws guidance error in development when request origin is not a tunnel', async () => {
+        process.env.NODE_ENV = 'development';
+
+        const customer = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request('https://localhost'),
+          waitUntil: vi.fn(),
+        });
+
+        try {
+          await customer.login();
+        } catch (error) {
+          const response = error as Response & {message?: string};
+
+          expect(response.status).toBe(400);
+          expect(response.message).toContain('--customer-account-push');
+          expect(response.message).toContain('.tryhydrogen.dev');
+        }


I think we should add expect.assertions(3) at the top of this test as a defensive measure. If the code under test were refactored to no longer throw, the catch block would never execute and the test would silently pass with zero assertions.

Suggested change

it('Throws guidance error in development when request origin is not a tunnel', async () => {

process.env.NODE_ENV = 'development';

const customer = createCustomerAccountClient({

session,

customerAccountId: 'customerAccountId',

shopId: '1',

request: new Request('https://localhost'),

waitUntil: vi.fn(),

});

try {

await customer.login();

} catch (error) {

const response = error as Response & {message?: string};

expect(response.status).toBe(400);

expect(response.message).toContain('--customer-account-push');

expect(response.message).toContain('.tryhydrogen.dev');

}

it('Throws guidance error in development when request origin is not a tunnel', async () => {

expect.assertions(3);

process.env.NODE_ENV = 'development';

const customer = createCustomerAccountClient({

session,

customerAccountId: 'customerAccountId',

shopId: '1',

request: new Request('https://localhost'),

waitUntil: vi.fn(),

});

try {

await customer.login();

} catch (error) {

const response = error as Response & {message?: string};

expect(response.status).toBe(400);

expect(response.message).toContain('--customer-account-push');

expect(response.message).toContain('.tryhydrogen.dev');

}

});

Same applies to the handleAuthStatus() test at line 1304. This is a pre-existing pattern throughout the file (~6 other tests use the same try/catch without expect.assertions()), so not unique to this PR - I'm fine with this being a follow-up as long as it gets done :)

kdaviduik · 2026-02-26T05:07:31Z

packages/hydrogen/src/customer/customer.ts

+      if (process.env.NODE_ENV === 'development') {
+        if (!requestUrl.hostname.endsWith('.tryhydrogen.dev')) {
+          throw new Response(
+            [
+              'Customer Account API OAuth requires a Hydrogen tunnel in local development.',
+              'Run `shopify hydrogen dev --customer-account-push`.',
+              'Then open the tunnel URL shown in your terminal (`https://*.tryhydrogen.dev`) instead of localhost.',
+            ].join('\n\n'),
+            {
+              status: 400,
+              headers: {
+                'Content-Type': 'text/plain; charset=utf-8',
+              },
+            },
+          );
+        }
+      }


(threading) Re: the customAuthStatusHandler asymmetry (lines 137-139) - this check in login() always fires regardless of whether a customAuthStatusHandler is provided, but the check in defaultAuthStatusHandler is bypassed when a custom handler is set.

non-blocking: This asymmetry is actually correct - login() initiates an OAuth redirect that fundamentally requires a publicly reachable URL, so the tunnel check is always relevant. A customAuthStatusHandler might intentionally handle the non-tunnel case differently (e.g., a mock or test double). I think a brief code comment explaining this rationale would help future readers.

i’ve added this check whenever ifInvalidCredentialThrowError() is placed. this seems to be the most reliable way: nowhere that needs a valid credential will work without it being tunnelled

do you think this is a good assumption?

yeah that sounds reasonable to me!

packages/hydrogen/src/customer/customer.ts

kdaviduik · 2026-02-26T05:26:07Z

packages/hydrogen/src/customer/customer.ts

+          throw new Response(
+            [
+              'Customer Account API OAuth requires a Hydrogen tunnel in local development.',
+              'Run `shopify hydrogen dev --customer-account-push`.',


note: these commands can also be aliased as just h2 dev rather than shopify hydrogen dev. i don't think the message needs to be updated but if you can think of a good way to communicate that either one works, go for it

thats very fair

i made it more generic just mentioning to use the flag. if they got to this screen it means they know how to start the dev server, and however they did, they can just add the flag to the end

yep exactly, i think that's super reasonable!

kdaviduik

i trust you, though i'm not able to 🎩 this - i'm seeing this error. both with mock shop and a shop linked. i will investigate more tomorrow - this is also happening to be on main

fredericoo · 2026-02-26T12:43:00Z

i trust you, though i'm not able to 🎩 this - i'm seeing this error. both with mock shop and a shop linked. i will investigate more tomorrow - this is also happening to be on main

yeah this is a real issue: if you are logged in or have a token you get a 500 instead, working on it!

binks-code-reviewer · 2026-02-26T16:10:07Z

🤖 Code Review · #projects-dev-ai for questions
React with 👍/👎 or reply — all feedback helps improve the agent.

✅ Complete - No issues

📋 History

✅ No issues → ✅ No issues → ✅ No issues

fredericoo · 2026-02-26T17:29:20Z

packages/hydrogen/src/customer/customer.ts

+function throwIfNotTunnelled(hostname: string) {
+  if (process.env.NODE_ENV === 'development') {


i’ve opted to place all the logic in this function instead of lots of ifs

when vite bundles this it’ll become a noop function so it is fine, although it adds a few bytes to the bundle we will survive

fredericoo · 2026-02-26T17:29:56Z

packages/hydrogen/src/customer/customer.ts

        process.env.NODE_ENV === 'production'
          ? 'Internal Server Error'
-          : 'You do not have the valid credential to use Customer Account API (/account).';
+          : 'You do not have a valid credential to use Customer Account API (/account).';


this read a little broken english, right?

agreed. "...have valid credentials..." reads even better to me. the singular "credential" feels a bit off to me

fredericoo · 2026-02-26T18:54:00Z

packages/hydrogen/src/customer/types.ts

  isLoggedIn: () => Promise<boolean>;
  /** Check for a not logged in customer and redirect customer to login page. The redirect can be overwritten with `customAuthStatusHandler` option. */
-  handleAuthStatus: () => void | DataFunctionValue;
+  handleAuthStatus: () => Promise<void>;


conversation starter:

this type was wrong. this function is async, and it does not return DataFunctionValue (which includes Response). those things are thrown today

I think i should leave this alone in this PR (revert this change) and we fix it later, with a changelog.

this also is impacting whenever we call handleAuthStatus() which is in many loaders within skeletons. they have a race condition: if they get processed in due time, the response (redirect) will be thrown, otherwise not

I think i should leave this alone in this PR (revert this change) and we fix it later, with a changelog.

nice catch and that sounds great! i'd definitely vote to fix it. i'm fine either way - whether you want to fix it in this PR or as a follow up. but if this gets fixed in this PR the changeset should definitely be updated to mention it

kdaviduik · 2026-02-27T04:29:11Z

packages/hydrogen/src/customer/customer.test.ts

+          await customer.login();
+        } catch (error) {
+          const response = error as Response & {message?: string};
+


non-blocking: these tests work correctly because the test file globally stubs Response with a mock class at lines 26-38 that stores the constructor's body argument as .message. This is a pre-existing test pattern, not something this PR introduced.

The tests are correct and pass as intended. However, .message is a non-standard property that only exists due to this mock - the standard Response Web API doesn't have it. If the test environment or mock strategy ever changes, these assertions would silently break. Using await response.text() would be the spec-compliant approach, but since this follows the existing pattern throughout the file, I'm fine with this shipping as-is.

kdaviduik · 2026-02-27T04:29:11Z

packages/hydrogen/src/customer/customer.ts


+function throwIfNotTunnelled(hostname: string) {
+  if (process.env.NODE_ENV === 'development') {
+    if (!hostname.endsWith('.tryhydrogen.dev')) {


non-blocking: I think a brief comment here would help future maintainers understand the coupling between this check and the tunnel infrastructure:

Suggested change

if (!hostname.endsWith('.tryhydrogen.dev')) {

// This domain must match the tunnel domain provisioned by --customer-account-push.

// If the tunnel infrastructure changes domains, update this check.

if (!hostname.endsWith('.tryhydrogen.dev')) {

Extracting to a named constant (e.g., TUNNEL_DOMAIN_SUFFIX) would also reduce the magic string, but a comment is sufficient for a single-use constant imo

kdaviduik · 2026-02-27T04:29:11Z

packages/hydrogen/src/customer/customer.ts

+    if (!hostname.endsWith('.tryhydrogen.dev')) {
+      throw new Response(
+        [
+          'Customer Account API OAuth requires a Hydrogen tunnel in local development.',


non-blocking, very minor polish: a developer seeing this error has NOT yet started with the --customer-account-push flag. The sequencing might read slightly more naturally as "Restart your development server with the --customer-account-push flag..." since the implicit step is "stop your current server, restart with the flag."

kdaviduik · 2026-02-27T06:13:10Z

packages/hydrogen/src/customer/customer.ts

+          throw new Response(
+            [
+              'Customer Account API OAuth requires a Hydrogen tunnel in local development.',
+              'Run `shopify hydrogen dev --customer-account-push`.',


yep exactly, i think that's super reasonable!

kdaviduik · 2026-02-27T06:13:47Z

packages/hydrogen/src/customer/customer.ts

+      if (process.env.NODE_ENV === 'development') {
+        if (!requestUrl.hostname.endsWith('.tryhydrogen.dev')) {
+          throw new Response(
+            [
+              'Customer Account API OAuth requires a Hydrogen tunnel in local development.',
+              'Run `shopify hydrogen dev --customer-account-push`.',
+              'Then open the tunnel URL shown in your terminal (`https://*.tryhydrogen.dev`) instead of localhost.',
+            ].join('\n\n'),
+            {
+              status: 400,
+              headers: {
+                'Content-Type': 'text/plain; charset=utf-8',
+              },
+            },
+          );
+        }
+      }


yeah that sounds reasonable to me!

kdaviduik · 2026-02-27T06:15:28Z

packages/hydrogen/src/customer/types.ts

  isLoggedIn: () => Promise<boolean>;
  /** Check for a not logged in customer and redirect customer to login page. The redirect can be overwritten with `customAuthStatusHandler` option. */
-  handleAuthStatus: () => void | DataFunctionValue;
+  handleAuthStatus: () => Promise<void>;


I think i should leave this alone in this PR (revert this change) and we fix it later, with a changelog.

nice catch and that sounds great! i'd definitely vote to fix it. i'm fine either way - whether you want to fix it in this PR or as a follow up. but if this gets fixed in this PR the changeset should definitely be updated to mention it

feat: error when using CAAPI untunelled

675d516

fredericoo marked this pull request as ready for review February 25, 2026 16:00

fredericoo requested a review from a team as a code owner February 25, 2026 16:00

kdaviduik approved these changes Feb 26, 2026

View reviewed changes

kdaviduik reviewed Feb 26, 2026

View reviewed changes

fredericoo changed the title ~~feat: error when using CAAPI untunelled~~ feat: error when using CAAPI untunnelled Feb 26, 2026

fix: check all caapi endpoints

bb47c9d

fredericoo commented Feb 26, 2026

View reviewed changes

fredericoo added 5 commits February 26, 2026 17:51

test: verify number of assertions

832eb61

test: verify number of assertions

66587ae

chore: more generic message

569c90f

fix: tunnel check missing

170215b

fix: type for handleAuthStatus

dceae08

fredericoo force-pushed the fb-better-feedback-for-cap branch from a681ba2 to dceae08 Compare February 26, 2026 18:49

fredericoo commented Feb 26, 2026

View reviewed changes

fredericoo requested a review from kdaviduik February 26, 2026 19:03

kdaviduik reviewed Feb 27, 2026

View reviewed changes

fredericoo changed the title ~~feat: error when using CAAPI untunnelled~~ feat: show a clear error when trying to use CAAPI untunnelled in localhost Feb 27, 2026

		function throwIfNotTunnelled(hostname: string) {
		if (process.env.NODE_ENV === 'development') {

Conversation

fredericoo commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

HOW to test your changes?

Post-merge steps

Checklist

Uh oh!

shopify bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kdaviduik left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kdaviduik left a comment

Choose a reason for hiding this comment

Uh oh!

fredericoo commented Feb 26, 2026

Uh oh!

binks-code-reviewer bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericoo Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fredericoo commented Feb 25, 2026 •

edited

Loading

shopify bot commented Feb 25, 2026 •

edited

Loading

kdaviduik left a comment •

edited

Loading

binks-code-reviewer bot commented Feb 26, 2026 •

edited

Loading

fredericoo Feb 26, 2026 •

edited

Loading