fix(astro): Do not inject withSentry into Cloudflare Pages#19558
fix(astro): Do not inject withSentry into Cloudflare Pages#19558
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (configFile.endsWith('.toml')) { | ||
| return content.includes('pages_build_output_dir'); | ||
| } |
There was a problem hiding this comment.
Bug: The isCloudflarePages function incorrectly detects Pages projects by checking for pages_build_output_dir in TOML file comments, leading to misidentification.
Severity: MEDIUM
Suggested Fix
To fix this, either implement TOML comment stripping before checking the file content, similar to how JSON/JSONC files are handled, or use a proper TOML parsing library to accurately read configuration keys. An alternative is to use a more reliable detection mechanism, like checking for Workers-specific fields.
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: packages/astro/src/integration/index.ts#L280-L282
Potential issue: The `isCloudflarePages` function checks for the presence of the string
`pages_build_output_dir` in `.toml` files using a simple `content.includes()` check.
This method does not account for TOML syntax, causing it to return a false positive if
the string appears within a comment. This misidentifies a Cloudflare Workers project as
a Pages project, which then prevents the `sentryCloudflareVitePlugin` from being added.
As a result, Sentry initialization for non-SSR routes (like actions and API routes) is
skipped, leading to incomplete error and performance monitoring for the affected Workers
project.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| try { | ||
| // Strip comments from JSONC before parsing | ||
| const parsed = JSON.parse(content.replace(/\/\*[\s\S]*?\*\/|\/\/.*/g, '')); |
There was a problem hiding this comment.
JSONC comment regex corrupts strings containing double slashes
Medium Severity
The regex /\/\*[\s\S]*?\*\/|\/\/.*/g used to strip comments before JSON.parse also matches // inside JSON string values (e.g., URLs like "https://api.example.com"). This corrupts the JSON, causes JSON.parse to throw, and the catch block returns false. A Cloudflare Pages project with any URL-containing value in wrangler.json or wrangler.jsonc would be misdetected as Workers, causing sentryCloudflareVitePlugin and withSentry wrapping to be unnecessarily added while the intended ssr.noExternal config is skipped. The regex is also needlessly applied to plain .json files which never have comments.
| const content = fs.readFileSync(configPath, 'utf-8'); | ||
|
|
||
| if (configFile.endsWith('.toml')) { | ||
| return content.includes('pages_build_output_dir'); |
There was a problem hiding this comment.
TOML detection matches comments containing the key name
Medium Severity
The TOML detection uses a naive content.includes('pages_build_output_dir') which matches the string anywhere — including TOML comments (e.g., # pages_build_output_dir = ...). A Workers project with a commented-out pages_build_output_dir line would be misdetected as Cloudflare Pages, causing the critical sentryCloudflareVitePlugin and withSentry wrapping to be skipped. This results in losing per-request isolation, async context, and trace propagation for the Worker.
size-limit report 📦
|
1 participant
When running on Cloudflare Pages the
withSentryfunction got bundled and wrapped into theindex.js. This actually didn't cause any problems during the runtime, but it just added unnecessary code into the bundle. This removes now the automatic wrapping, which is required for Cloudflare Workers only.By adding the plugin
sentryCloudflareNodeWarningPluginwe also remove tons of warnings like following when building for Cloudflare Pages:Closes #19559 (added automatically)