[Bug]: Static Ngrok URLs no longer work

ChaseWNorton commented 2 years ago

Please confirm that you have:

[X] Searched existing issues to see if your issue is a duplicate. (If you’ve found a duplicate issue, feel free to add additional information in a comment on it.)
[X] Reproduced the issue in the latest CLI version.

In which of these areas are you experiencing a problem?

App

Expected behavior

Navigate to the static ngrok urls I've been using for dev in the past and everything tunnel correctly

Actual behavior

Overrides tunneling. Even though by default it uses ngrok, it suggests cloudflared as the approved method. Conflicting documentation and implementation.

Stack trace

No response

Reproduction steps

ngrok start configname
npm run dev
or npm run dev -- --tunnel-url ngrokURL
overrides and fails

Operating System

MAC OS

Shopify CLI version (check your project's `package.json` if you're not sure)

3

Shell

No response

Node version (run `node -v` if you're not sure)

No response

What language and version are you using in your application?

No response

gonzaloriestra commented 2 years ago

Hi! Thanks for creating the issue.

Some follow-up questions:

What do you mean by overrides and fails? It creates a new tunnel and ignores the one you are passing?
Could you provide a stack trace or screenshot?
Are you passing the port as well? It should be something like npm run dev -- --tunnel-url https://b54c-87-220-76-32.eu.ngrok.io:3000

Regarding Ngrok vs Cloudflare, we have found some issues with Ngrok lately and have plans to better integrate with Cloudflare, that's why we recommend that if you don't want to use the internal Ngrok solution.

awd commented 2 years ago

It sounds very similar if not the same issue I mentioned here: https://github.com/Shopify/cli/issues/118#issuecomment-1176575176

The ngrok tunnel is trying to serve both a checkout ui extension (dev tools) + web app. There is no way to disable one or the other, so the web app cannot ever be served through ngrok.

@gonzaloriestra - what type of test repo are you testing this against? Does it include both (at least) a web app and some an extension which are trying to serve dev-tools / ui?

CC @alvaro-shopify

pepicrft commented 2 years ago

Hi @awd, Checking it here. The CLI has a reverse proxy to do path-based routing and ensure that the HTTP and websocket requests reach the extensions and the web servers. Do you mind sharing the logs that you see when you invoke the CLI with --verbose? Remember to hide any sensitive information from it.

wisniewski94 commented 2 years ago

The problem is with

export async function runConcurrentHTTPProcessesAndPathForwardTraffic(portNumber = undefined, proxyTargets, additionalProcesses) {
    const rules = {};
    const processes = await Promise.all(proxyTargets.map(async (target) => {
        const targetPort = await port.getRandomPort();
        rules[target.pathPrefix ?? '/'] = `http://localhost:${targetPort}`;
        return {
            prefix: target.logPrefix,
            action: async (stdout, stderr, signal) => {
                await target.action(stdout, stderr, signal, targetPort);
            },
        };
    }));
    const availablePort = portNumber ?? (await port.getRandomPort());
    output.debug(output.content `
Starting reverse HTTP proxy on port ${output.token.raw(availablePort.toString())}
Routing traffic rules:
${output.token.json(JSON.stringify(rules))}
`);
    const proxy = httpProxy.createProxy();
    const server = http.createServer(function (req, res) {
        const target = match(rules, req);
        if (target)
            return proxy.web(req, res, { target });
        output.debug(`
Reverse HTTP proxy error - Invalid path: ${req.url}
These are the allowed paths:
${output.token.json(JSON.stringify(rules))}
`);
        res.statusCode = 500;
        res.end(`Invalid path ${req.url}`);
    });
    // Capture websocket requests and forward them to the proxy
    server.on('upgrade', function (req, socket, head) {
        const target = match(rules, req);
        if (target)
            return proxy.ws(req, socket, head, { target });
        socket.destroy();
    });
    await Promise.all([
        output.concurrent([...processes, ...additionalProcesses], (abortSignal) => {
            abortSignal.addEventListener('abort', async () => {
                await server.close();
            });
        }),
        server.listen(availablePort),
    ]);
}

portNumber parameter is never used. Http reverse proxy always generates new random port. That means domain should be fine but the port is the part that is wrong.

Also @pepicrft --verbose will output wrong information as logging function uses portNumber while it should log targetPort

I will create PR for it soon

cc gonzaloriestra

isaacroldan commented 2 years ago

Discussed with @wisniewski94 in his PR: the issue is not in the runConcurrentHTTPProcessesAndPathForwardTraffic function :)

Please when using a custom tunnel, make sure that:

The tunnel (ngrok, cloudflare or other) process is running
That you specify the port where the tunnel is redirecting the traffic, like this: yarn dev --tunnel-url https://{my-tunnel}.com:{tunnel-port}

real example:

> ngrok http 8123
> yarn dev --tunnel-url https://000-79-116-29-0.ngrok.io:8123

cjohansson commented 2 years ago

Same issue for me but on Ubuntu 22

yarn dev --tunnel-url https://b6e0d58406ce.ngrok.io:49169      
yarn run v1.21.1
$ shopify app dev --tunnel-url https://b6e0d58406ce.ngrok.io:49169
✔ Dependencies installed

Using your previous dev settings:
- Org:          MASKED
- App:          MASKED
- Dev store:    MASKED
- Update URLs:  Always

To reset your default dev config, run npm run dev -- --reset

✔ URL updated

Shareable app URL

  https://b6e0d58406ce.ngrok.io?shop=MASKED&host=MASKED

node:events:491
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE: address already in use :::49169
    at Server.setupListenHandle [as _listen2] (node:net:1432:16)
    at listenInCluster (node:net:1480:12)
    at Server.listen (node:net:1568:7)
    at runConcurrentHTTPProcessesAndPathForwardTraffic (file:///MASKED/node_modules/@shopify/app/dist/cli/utilities/app/http-reverse-proxy.js:57:16)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async dev (file:///MASKED/node_modules/@shopify/app/dist/cli/services/dev.js:108:9)
    at async Dev.run (file:///MASKED/node_modules/@shopify/app/dist/cli/commands/app/dev.js:17:9)
    at async Dev._run (/MASKED/node_modules/@oclif/core/lib/command.js:67:22)
    at async Config.runCommand (/MASKED/node_modules/@oclif/core/lib/config/config.js:268:25)
    at async run (/MASKED/node_modules/@oclif/core/lib/main.js:78:5)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1459:8)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  code: 'EADDRINUSE',
  errno: -98,
  syscall: 'listen',
  address: '::',
  port: 49169
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

If I instead run with npm like the documentation says I get

npm run dev --tunnel-url https://b6e0d58406ce.ngrok.io:49169

> MASKED@1.0.0 dev
> shopify app dev "https://b6e0d58406ce.ngrok.io:49169"

━━━━━━ Error ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
    command app:dev:https://b6e0d58406ce.ngrok.io:49169 not found

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

isaacroldan commented 2 years ago

Hi @cjohansson ! Can I ask how did you start your tunnel? did you do ngrok http 49169? Seems like that address is in use... could there be any other process running in that port?

As for the issue with npm, it works a bit different so the correct command would be: npm run dev -- --tunnel-url ... (notice the extra --)

cjohansson commented 2 years ago

@isaacroldan ok I have a DDEV-LOCAL environment running (like an orchestrated docker environment) for the backend already listening to the port and the ngrok URL, I don't understand why Shopify-cli needs to start another webserver and allocate a new ngrok URL, is it possible to bypass that operation and still use the shopify-cli for setting up the app in the Shopify API?

rsdavis commented 2 years ago

Hi @awd, Checking it here. The CLI has a reverse proxy to do path-based routing and ensure that the HTTP and websocket requests reach the extensions and the web servers. Do you mind sharing the logs that you see when you invoke the CLI with --verbose? Remember to hide any sensitive information from it.

@pepicrft Thanks for pointing this out. I suspected there was another proxy somewhere when I noticed that the ngrok tunnel setup by shopify-cli was pointing to a port that was not associated with the backend or frontend. Can you explain more why this is here and how it works? Where are the rules for path routing? Or maybe you could point me to some explanation? I could not find any documentation on how this works and it feels a bit magic to me, thanks.

isaacroldan commented 2 years ago

Hi @cjohansson , the tunnel is needed if you have extensions (the CLI spins up an extension server). If you don't have extensions you can avoid the tunnel using the flag --no-tunnel, this will make your frontend app available directly from localhost. Take into account that this flag is still experimental and can cause other issues, depending on the configuration of your project.

@rsdavis The proxy has two main routes: /extensions that redirects to the CLI extension server (if you have extensions) and / that catches everything else and redirects it to your app.

cjohansson commented 2 years ago

@isaacroldan I had no luck with that, it seems to start a web-server anyway, if I could just pass a URL to the shopify-cli and it would update my app settings that would be great and it wouldn't start a web-server

npm run dev -- --no-tunnel

> shipit@1.0.0 dev
> shopify app dev "--no-tunnel"

✔ Dependencies installed

Using your previous dev settings:
- Org:          MASKED
- App:          MASKED
- Dev store:    MASKED
- Update URLs:  Always

To reset your default dev config, run npm run dev -- --reset

✔ URL updated

App URL

  http://localhost:34009?shop=MASKED.myshopify.com&host=MASKED

backend  | 1
frontend | 
frontend | > dev
frontend | > vite
frontend | 
frontend | 
frontend |   vite v2.9.15 dev server running at:
frontend | 
frontend |   > Local:    http://localhost:34009/
frontend |   > Network:  http://192.168.50.9:34009/
frontend |   > Network:  http://172.22.0.1:34009/
frontend |   > Network:  http://172.29.0.1:34009/
frontend | 
frontend |   ready in 347ms.
frontend |

After visiting the URL in the browser:

frontend | 1:39:00 PM [vite] http proxy error:
frontend | Error: connect ECONNREFUSED 127.0.0.1:43541
frontend |     at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1247:16)

isaacroldan commented 2 years ago

Hi @cjohansson , not sure I understand what you need 🤔 Using --no-tunnel we don't create a proxy. In your example the port 34009 is the port of your frontend app. And 43541 would be the port of your backend (both random)

Do you mean you don't want the frontend and want to connect directly to the backend? Could you explain me a bit more of your use case so that I can understand it better? Thanks!

cjohansson commented 2 years ago

I just want a working development environment for an app, when I follow the instructions for creating an app using the shopify-cli (PHP) version I only stumble upon these errors, the only way to get it working is to not use the dev command at all, using the build command I can get an development environment working, but maybe it is an issue related to the PHP-version only.

I am not sure what the command is supposed to do, documentation says "Start your development server" but it seems to do a lot of other things as well, like connecting to Shopify Partners and also like it is starting multiple servers? One for backend and one for frontend on different ports? Usually a webpage only has one webserver and one port taking requests, I don't understand why there is two servers? If I could just pass the URL to my already existing webserver that has a public reachable URL that would be perfect

cjohansson commented 2 years ago

If the frontend server is like a watcher that will rebuild when I do changes it would be great if I could pass a URL for my backend server already existing and the CLI would only start the frontend server

isaacroldan commented 2 years ago

Hi @cjohansson , I just created a new PHP app and it worked. To clarify some concepts, when you create a new PHP app, inside your web folder there is the PHP code but also a frontend folder with some React code (the file web/frontend/pages/index.jsx will be the one rendered on your admin when you install the app on a store.

The CLI creates a proxy server to redirect traffic, but this is not relevant. When executing npm run dev the CLI takes care of initiating both the PHP and the React processes, you don't need to do that manually. The CLI will then expose a url and a port pointing directly to your frontend (and your frontend communicates with the PHP backend).

I recommend not using the --no-tunnel flag as it is in beta and might not work properly, the CLI will create an ngrok tunnel pointing to your frontend.

From your previous logs I can see that the CLI is not loading the backend part (PHP) correctly, might be because you ran it manually?

Hope this helps!

cjohansson commented 2 years ago

ok thanks @isaacroldan for the patience, I will try again tomorrow. Does the PHP backend use a PHP runtime in a docker container created with the Dockerfile or does the backend PHP run from the local systems installed PHP runtime? I was thinking maybe my locally installed PHP might be the wrong version for the app

cjohansson commented 2 years ago

Ok I tried it again, steps on this page up till step 2 work but then when you start following steps on this page it wants you to run PHP scripts using the systems built-in PHP runtime instead of inside the Docker-container and it does not work on my system, these commands composer install, php artisan key:generate and php artisan migrate should be executed inside the Docker-container

I tried running the entire npm run dev inside a Docker-container but it won't work because it can't detect the proper URL and can't connect to the system browser

cjohansson commented 2 years ago

Output from step 3


npm run dev

> MASKED@1.0.0 dev
> shopify app dev

✔ Dependencies installed

To run this command, log in to Shopify Partners.
👉 Press any key to open the login page on your browser
✔ Logged in.

Looks like this is the first time you're running dev for this project.
Configure your preferences by answering a few questions.

Before you preview your work, it needs to be associated with an app.

✔ Create this project as a new app on Shopify? · Yes, create it as a new app
✔ App Name · MASKED
✅ Success! MASKED has been created on your Partners account.
✔ Which development store would you like to use to view your project? · SHIPIT testing christian
✅ Success! Converted MASKED.myshopify.com to a Test store.
✅ Success! The tunnel is running and you can now view your app.

For your convenience, we've given your app a default URL: https://55ca60d2d40e.ngrok.io.

You can update your app's URL anytime in the Partners Dashboard. But once your app is live, updating its URL will disrupt merchant access.

Shareable app URL

  https://55ca60d2d40e.ngrok.io?shop=MASKED.myshopify.com&host=MASKED

2022-10-20 06:19:38 | backend  | PHP Deprecated:  Return type of Symfony\Component\Console\Helper\HelperSet::getIterator() should either 
                                 be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] 
                                 attribute should be used to temporarily suppress the notice in 
                                 phar:///usr/bin/composer.phar/vendor/symfony/console/Helper/HelperSet.php on line 112
2022-10-20 06:19:38 | backend  |
2022-10-20 06:19:38 | backend  |
2022-10-20 06:19:38 | backend  | Deprecated: Return type of Symfony\Component\Console\Helper\HelperSet::getIterator() should either be 
                                 compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute 
                                 should be used to temporarily suppress the notice in 
                                 phar:///usr/bin/composer.phar/vendor/symfony/console/Helper/HelperSet.php on line 112
2022-10-20 06:19:38 | backend  |
2022-10-20 06:19:38 | backend  | Deprecation Notice: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in phar
                                 :///usr/bin/composer.phar/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:48
2022-10-20 06:19:38 | backend  | Deprecation Notice: Return type of Composer\Repository\ArrayRepository::count() should either be 
                                 compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to 
                                 temporarily suppress the notice in 
                                 phar:///usr/bin/composer.phar/src/Composer/Repository/ArrayRepository.php:206
2022-10-20 06:19:38 | backend  | Deprecation Notice: Return type of Composer\Repository\ArrayRepository::count() should either be 
                                 compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to 
                                 temporarily suppress the notice in 
                                 phar:///usr/bin/composer.phar/src/Composer/Repository/ArrayRepository.php:206
2022-10-20 06:19:38 | backend  | Deprecation Notice: Return type of Composer\Repository\ArrayRepository::count() should either be 
                                 compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to 
                                 temporarily suppress the notice in 
                                 phar:///usr/bin/composer.phar/src/Composer/Repository/ArrayRepository.php:206
2022-10-20 06:19:38 | backend  |
2022-10-20 06:19:38 | backend  | Deprecation Notice: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in phar
                                 :///usr/bin/composer.phar/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:48
2022-10-20 06:19:38 | backend  |
2022-10-20 06:19:39 | backend  | > Composer\Config::disableProcessTimeout
2022-10-20 06:19:39 | backend  |
2022-10-20 06:19:39 | backend  | > php artisan serve
2022-10-20 06:19:39 | backend  |
2022-10-20 06:19:39 | backend  | PHP Warning:  require(/MASKED/MASKED/web/vendor/autoload.php): 
                                 Failed to open stream: No such file or directory in 
                                 /MASKED/MASKED/web/artisan on line 18
2022-10-20 06:19:39 | backend  | PHP Fatal error:  Uncaught Error: Failed opening required 
                                 '/MASKED/MASKED/web/vendor/autoload.php' 
                                 (include_path='.:/usr/share/php') in 
                                 /MASKED/MASKED/web/artisan:18
2022-10-20 06:19:39 | backend  | Stack trace:
2022-10-20 06:19:39 | backend  | #0 {main}
2022-10-20 06:19:39 | backend  |   thrown in /MASKED/MASKED/web/artisan on line 18
2022-10-20 06:19:39 | backend  |
2022-10-20 06:19:39 | backend  | Script php artisan serve handling the serve event returned with error code 255
2022-10-20 06:19:39 | backend  |

/MASKED/MASKED/node_modules/yoga-layout-prebuilt/yoga-layout/build/Release/nbind.js:53
        throw ex;
        ^

Abort [Error]: Command failed with exit code 255: composer serve
PHP Deprecated:  Return type of Symfony\Component\Console\Helper\HelperSet::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/bin/composer.phar/vendor/symfony/console/Helper/HelperSet.php on line 112
Deprecation Notice: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in phar:///usr/bin/composer.phar/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:48
Deprecation Notice: Return type of Composer\Repository\ArrayRepository::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/bin/composer.phar/src/Composer/Repository/ArrayRepository.php:206
Deprecation Notice: Return type of Composer\Repository\ArrayRepository::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/bin/composer.phar/src/Composer/Repository/ArrayRepository.php:206
Deprecation Notice: Return type of Composer\Repository\ArrayRepository::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/bin/composer.phar/src/Composer/Repository/ArrayRepository.php:206
Deprecation Notice: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in phar:///usr/bin/composer.phar/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:48
> Composer\Config::disableProcessTimeout
> php artisan serve
PHP Warning:  require(/MASKED/MASKED/web/vendor/autoload.php): Failed to open stream: No such file or directory in /MASKED/MASKED/web/artisan on line 18
PHP Fatal error:  Uncaught Error: Failed opening required '/MASKED/MASKED/web/vendor/autoload.php' (include_path='.:/usr/share/php') in /MASKED/MASKED/web/artisan:18
Stack trace:
#0 {main}
  thrown in /MASKED/MASKED/web/artisan on line 18
Script php artisan serve handling the serve event returned with error code 255

Deprecated: Return type of Symfony\Component\Console\Helper\HelperSet::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/bin/composer.phar/vendor/symfony/console/Helper/HelperSet.php on line 112
    at makeError (file:///MASKED/MASKED/node_modules/execa/lib/error.js:59:11)
    at handlePromise (file:///MASKED/MASKED/node_modules/execa/index.js:119:26)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Module.exec (file:///MASKED/MASKED/node_modules/@shopify/cli-kit/dist/system.js:32:9)
    at async Object.action (file:///MASKED/MASKED/node_modules/@shopify/app/dist/cli/services/dev.js:182:13)
    at async file:///MASKED/MASKED/node_modules/@shopify/cli-kit/dist/private/node/ui/components/ConcurrentOutput.js:70:17
    at async Promise.all (index 1)
    at async runProcesses (file:///MASKED/MASKED/node_modules/@shopify/cli-kit/dist/private/node/ui/components/ConcurrentOutput.js:67:13) {
  tryMessage: null,
  type: 0
}
``

cjohansson commented 2 years ago

I managed to setup the Docker-container for PHP with the command

docker build --build-arg SHOPIFY_API_KEY=MASKED -t "NAME:Dockerfile" .

which actually runs all commands listead on this page

But when running npm run dev the PHP commands are still not executed inside the Docker-container so I get same errors as before, somehow the npm run dev command should use the Docker-container for the PHP version instead of running the commands directly on the system

pepicrft commented 2 years ago

the PHP commands are still not executed inside the Docker-container so I get same errors as before

npm run dev runs the command from the shopify.web.toml in the same environment where the CLI is running. If you would like to run a command inside a Docker container, you can adjust that command to be something along the lines of:

[commands]
dev = "docker run ..."

Shopify / cli