[Bug]: Cloudflared binary postinstall script - fail on version check

appleseed-iii commented 10 months 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, Extension

Expected behavior

on a Mac M3 the line 66 of @shopify/plugin-cloudflare/scripts/postinstall.js fails.

const versionArray = execFileSync(binTarget, ['--version'], {encoding: 'utf8'}).split(' ');

I've tested this on a Mac M1 & it works fine.

Actual behavior

const versionArray = execFileSync(binTarget, ['--version'], {encoding: 'utf8'}).split(' ') should return the versionArray; however, it fails.

Verbose output

shopify/plugin-cloudflare@3.53.0 postinstall /.../shopify-test/extensive-policy-app/node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare
> node scripts/postinstall.js

node:internal/errors:563
    ErrorCaptureStackTrace(err);
    ^

<ref *1> Error: spawnSync /.../shopify-test/extensive-policy-app/node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/bin/cloudflared Unknown system error -86
    at Object.spawnSync (node:internal/child_process:1124:20)
    at spawnSync (node:child_process:876:24)
    at execFileSync (node:child_process:919:15)
    at install (file:///.../shopify-test/extensive-policy-app/node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/scripts/postinstall.js:66:26)
    at file:///.../shopify-test/extensive-policy-app/node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/scripts/postinstall.js:120:1
    at ModuleJob.run (node:internal/modules/esm/module_job:218:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:329:24)
    at async loadESM (node:internal/process/esm_loader:28:7)
    at async handleMainPromise (node:internal/modules/run_main:113:12) {
  errno: -86,
  code: 'Unknown system error -86',
  syscall: 'spawnSync /.../shopify-test/extensive-policy-app/node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/bin/cloudflared',
  path: '/.../shopify-test/extensive-policy-app/node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/bin/cloudflared',
  spawnargs: [ '--version' ],
  error: [Circular *1],
  status: null,
  signal: null,
  output: null,
  pid: 0,
  stdout: null,
  stderr: null
}

Reproduction steps

follow shopify's getting started steps with pnpm to create the sample remix app https://shopify.dev/docs/apps/getting-started/create
after app setup pnpm i works unless postinstall scripts run. On the re-run of the plugin-cloudflare postinstall script the execFileSync fails. One reason postinstall scripts might run is if .npmrc includes node-linker = hoisted
Regardless of the configuration noted in step 2 above you can also recreate this issue by manually re-running the plugin-cloudflare postinstall script so that the version check runs (and fails :sob)

Operating System

Mac OS Sonoma 14.2.1

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

3.53.0

Shell

zsh

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

v20.11.0

What language and version are you using in your application?

Node

appleseed-iii commented 10 months ago

I also put a re-creation here:

https://github.com/appleseed-iii/shopify-cli-postinstall

gonzaloriestra commented 10 months ago

Hi!

That postinstall script basically downloads the Cloudlfared binary and I guess that something went wrong there. Have you tried deleting the node_modules folder (where the binary is downloaded) and reinstalling the dependencies? So that the process starts from scratch.

Other ideas:

Use a global installation of Cloudflared: brew install cloudflared and then SHOPIFY_CLI_CLOUDFLARED_PATH=/opt/homebrew/bin/cloudflared npm run dev
Create a tunnel in another tab (with Cloudflare, Ngrok...) and pass the URL with --tunnel-url. Something like: npm run dev -- --tunnel-url https://your-tunnel.ngrok.com:3000. More info: https://shopify.dev/docs/apps/tools/cli/commands#dev-flags

Let me know if it helps!

appleseed-iii commented 10 months ago

hey thanks. Your three solutions and what I found:

delete node_modules and reinstall - technically this just gets me to the same result. Namely, the initial install is fine bc it never hits line 66 of @shopify/plugin-cloudflare/scripts/postinstall.js. It's just when the postinstall script re-runs that line 66 hits, cloudflared already exists, so line 66 fails.
install cloudflared from brew & pass it through env... this one actually works & gets past line 66 no problem.
i didn't mess with this one.

So are the cloudflared from homebrew & the cloudflared in @shopify/plugin-cloudflare different versions?

appleseed-iii commented 10 months ago

yea looks like shopify is using 2023.5.1

but homebrew is using 2024.1.3

gonzaloriestra commented 10 months ago

More checks:

Does it work if you go to your app folder and run ./node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/bin/cloudflared --version?
Could you try running softwareupdate --install-rosetta? That installs Rosetta 2, required to run x64 executables in ARM

appleseed-iii commented 10 months ago

@gonzaloriestra first one:

% ./node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/bin/cloudflared --version
# => zsh: bad CPU type in executable: ./node_modules/.pnpm/@shopify+plugin-cloudflare@3.53.0_@types+node@20.11.0_@types+react@18.2.47_typescript@5.3.3/node_modules/@shopify/plugin-cloudflare/bin/cloudflared

i'll do the second in just a bit

appleseed-iii commented 10 months ago

ok, rosetta fixes the issue completely. nice i wasn't aware of that

gonzaloriestra commented 10 months ago

Great!

Cloudflare isn't generating ARM releases for some reason, so we need to use the x64 one. It works perfectly with Rosetta, which is suggested by MacOS when required, so most people already have it.

Anyway, we should improve the error message. I see we already have one for this situation, but it seems we are not checking that in the postinstall script. @isaacroldan you added it, just in case you have more info.

isaacroldan commented 10 months ago

That's a good catch, we check this specific error only when you try to run the tunnel (because we assume the first time the install always work). We should check for this error also when running the --version command.

Shopify / cli