Skip to content
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

🐛 BUG: Invalid define value (must be an entity name or valid JSON syntax) #5720

Closed
flexchar opened this issue Apr 27, 2024 · 7 comments
Closed
Labels
bug Something that isn't working needs reproduction Needs reproduction from OP

Comments

@flexchar
Copy link

flexchar commented Apr 27, 2024

Which Cloudflare product(s) does this pertain to?

Wrangler core

What version(s) of the tool(s) are you using?

3.52.0

What version of Node are you using?

21

What operating system and version are you using?

Mac

Describe the Bug

I've reviewed the following actions and can not figure out why my input keeps getting rejected.

Observed behavior

Work. This is going to be huge in getting Sentry work seamlessly with source map upload.

Expected behavior

 ⛅️ wrangler 3.52.0
-------------------

✘ [ERROR] Invalid define value (must be an entity name or valid JSON syntax): abc123


✘ [ERROR] Build failed with 1 error:

  error: Invalid define value (must be an entity name or valid JSON syntax): abc123


🪵  Logs were written to ...

Steps to reproduce

Neither of these work:

export SENTRY_RELEASE=$(git rev-parse --short HEAD)
npx wrangler deploy src/routes.ts --define SENTRY_RELEASE:$SENTRY_RELEASE --outdir dist --dry-run
npx wrangler deploy src/routes.ts --define SENTRY_RELEASE:"$SENTRY_RELEASE" --outdir dist --dry-run

npx wrangler deploy src/routes.ts --define SENTRY_RELEASE:'$(git rev-parse HEAD)' --outdir dist --dry-run
npx wrangler deploy src/routes.ts --define SENTRY_RELEASE:$(git rev-parse HEAD) --outdir dist --dry-run

This is exactly what documentation says thou.

Please provide a link to a minimal reproduction

No response

Please provide any relevant error logs

No response

@flexchar flexchar added the bug Something that isn't working label Apr 27, 2024
@KianNH
Copy link
Contributor

KianNH commented Apr 27, 2024

You want --define SENTRY_RELEASE:'"$SENTRY_RELEASE"', it needs to be surrounded in two sets of quotes.

@flexchar
Copy link
Author

flexchar commented Apr 28, 2024

Actually it doesn't work. It does prevent script error but the result I'm getting "$SENTRY_RELEASE" inside the code. Aka, it doesn't evaluate the contents of the variable.

@penalosa
Copy link
Contributor

penalosa commented May 2, 2024

Thanks for reporting this! Would you be able to provide a reproduction repo that demonstrates the issue?

Running npx wrangler deploy src/routes.ts --define SENTRY_RELEASE:$(git rev-parse HEAD) --outdir dist --dry-run seems to work for me.

@penalosa penalosa added the needs reproduction Needs reproduction from OP label May 2, 2024
@flexchar
Copy link
Author

flexchar commented May 2, 2024

Of course, @penalosa, I am very curious why it is working for you but not for me.

Here is the reproducible with the fresh init:
https://github.com/flexchar/workers-issue-5720

hope that helps!

@flexchar
Copy link
Author

I know you guys are very busy, @penalosa and @KianNH, and I respect that. Nevertheless I wanted to give a little bump up here hoping you would find a chance for this in your near future sprints.

This would save the users of Workers Platform a lot of time enabling the source maps to be published for tools like Sentry. Currently debugging deployed (and minified) workers is like debugging a black box.

@penalosa
Copy link
Contributor

@flexchar It looks like this only works without quotes when the commit hash starts with an alphabetic character (if it starts with a number esbuild gets confused). I think this is why it was working for me initially.

Could you try the following script? That should ensure that the variable is treated as a string.

"scripts": {
    "test": "wrangler deploy src/index.ts --define \"SENTRY_RELEASE:'$(git rev-parse HEAD)'\" --outdir dist --dry-run"
  }

@penalosa
Copy link
Contributor

We haven't heard from you in while so I'm going to close this issue for now. If you're still running into problems feel free to comment with more details and we can investigate further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working needs reproduction Needs reproduction from OP
Projects
None yet
Development

No branches or pull requests

3 participants