-
-
Notifications
You must be signed in to change notification settings - Fork 77
feat(cloudflare): Support wrapping workers main file #1156
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
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Support wrapping workers main file ([#1156](https://github.com/getsentry/sentry-wizard/pull/1156))If none of the above apply, you can opt out of this check by adding |
s1gr1d
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition and good tests! Did you try the CLI locally in an actual project as well?
Surely, with couple of |
| ); | ||
| } catch (e) { | ||
| clack.log.warn( | ||
| 'Could not automatically set up Sentry initialization. Please set it up manually using instructions from https://docs.sentry.io/platforms/javascript/guides/cloudflare/', | ||
| ); | ||
| debug(e); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
4e2a379 to
29656bd
Compare
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Cloudflare
Mcp
Bug Fixes 🐛
Build / dependencies / internal 🔧Deps
Test
Other
Other
🤖 This preview updates automatically when you update the PR. |
| const entryPointPath = path.join(process.cwd(), entryPointFromConfig); | ||
|
|
||
| if (fs.existsSync(entryPointPath)) { | ||
| clack.log.info( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: For new projects, the wizard creates a wrangler.jsonc pointing to a non-existent entry point, then createSentryInitFile silently fails, leaving the project in a broken state.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
In a fresh project without a wrangler.jsonc, the wizard first calls ensureWranglerConfig(), which creates a config file pointing to a non-existent entry point like src/index.ts. Subsequently, createSentryInitFile() is called. It reads this entry point, but because the file doesn't exist (fs.existsSync() returns false), the function silently returns without creating the necessary Sentry initialization file. The wizard completes successfully, but the wrangler.jsonc remains pointing to a non-existent file, causing subsequent wrangler deploy commands to fail.
💡 Suggested Fix
The createSentryInitFile function should handle the case where the entry point file does not exist. Instead of silently returning, it should create the entry point file with the Sentry wrapper. This ensures the file pointed to by wrangler.jsonc actually exists and is correctly configured.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/cloudflare/sdk-setup.ts#L52-L55
Potential issue: In a fresh project without a `wrangler.jsonc`, the wizard first calls
`ensureWranglerConfig()`, which creates a config file pointing to a non-existent entry
point like `src/index.ts`. Subsequently, `createSentryInitFile()` is called. It reads
this entry point, but because the file doesn't exist (`fs.existsSync()` returns false),
the function silently returns without creating the necessary Sentry initialization file.
The wizard completes successfully, but the `wrangler.jsonc` remains pointing to a
non-existent file, causing subsequent `wrangler deploy` commands to fail.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8325139
(closes #1155)
(closes WIZARD-115)
This adds support for the Wizard to automatically wrap the default export of the Cloudflare Workers with
Sentry.withSentry.As of now no support for e.g. logging has been added (so the template stayed the same as the printed one from before).
The traceStep "Update Wrangler config with Sentry requirements" has moved down, as it will create a default entry file automatically if there is none, and will then update the
wrangler.jsoncwith the correct main file in case that was not given just yet. This is actually an edge case but a possibility.