Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions packages/astro/src/integration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
}

const isCloudflare = config?.adapter?.name?.startsWith('@astrojs/cloudflare');
const isCloudflareWorkers = isCloudflare && !isCloudflarePages();

if (isCloudflare) {
try {
Expand Down Expand Up @@ -191,8 +192,8 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
injectScript('page-ssr', buildServerSnippet(options || {}));
}

if (isCloudflare && command !== 'dev') {
// For Cloudflare production builds, additionally use a Vite plugin to:
if (isCloudflareWorkers && command !== 'dev') {
// For Cloudflare Workers production builds, additionally use a Vite plugin to:
// 1. Import the server config at the Worker entry level (so Sentry.init() runs
// for ALL requests, not just SSR pages — covers actions and API routes)
// 2. Wrap the default export with `withSentry` from @sentry/cloudflare for
Expand All @@ -215,6 +216,7 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
// Ref: https://developers.cloudflare.com/workers/runtime-apis/nodejs/
updateConfig({
vite: {
plugins: [sentryCloudflareNodeWarningPlugin()],
ssr: {
// @sentry/node is required in case we have 2 different @sentry/node
// packages installed in the same project.
Expand Down Expand Up @@ -255,6 +257,42 @@ function findDefaultSdkInitFile(type: 'server' | 'client'): string | undefined {
.find(filename => fs.existsSync(filename));
}

/**
* Detects if the project is a Cloudflare Pages project by checking for
* `pages_build_output_dir` in the wrangler configuration file.
*
* Cloudflare Pages projects use `pages_build_output_dir` while Workers projects
* use `assets.directory` or `main` fields instead.
*/
function isCloudflarePages(): boolean {
const cwd = process.cwd();
const configFiles = ['wrangler.jsonc', 'wrangler.json', 'wrangler.toml'];

for (const configFile of configFiles) {
const configPath = path.join(cwd, configFile);

if (!fs.existsSync(configPath)) {
continue;
}

const content = fs.readFileSync(configPath, 'utf-8');

if (configFile.endsWith('.toml')) {
return content.includes('pages_build_output_dir');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

}
Comment on lines +280 to +282
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


try {
// Strip comments from JSONC before parsing
const parsed = JSON.parse(content.replace(/\/\*[\s\S]*?\*\/|\/\/.*/g, ''));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

return 'pages_build_output_dir' in parsed;
} catch {
return false;
}
}

return false;
}

function getSourcemapsAssetsGlob(config: AstroConfig): string {
// The vercel adapter puts the output into its .vercel directory
// However, the way this adapter is written, the config.outDir value is update too late for
Expand Down
252 changes: 252 additions & 0 deletions packages/astro/test/integration/cloudflare.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,252 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { sentryAstro } from '../../src/integration';

const getWranglerConfig = vi.hoisted(() => vi.fn());

vi.mock('fs', async requireActual => {
return {
...(await requireActual<any>()),
existsSync: vi.fn((p: string) => {
const wranglerConfig = getWranglerConfig();

if (wranglerConfig && p.includes(wranglerConfig.filename)) {
return true;
}
return false;
}),
readFileSync: vi.fn(() => {
const wranglerConfig = getWranglerConfig();

if (wranglerConfig) {
return wranglerConfig.content;
}
return '';
}),
};
});

vi.mock('@sentry/vite-plugin', () => ({
sentryVitePlugin: vi.fn(() => 'sentryVitePlugin'),
}));

vi.mock('../../src/integration/cloudflare', () => ({
sentryCloudflareNodeWarningPlugin: vi.fn(() => 'sentryCloudflareNodeWarningPlugin'),
sentryCloudflareVitePlugin: vi.fn(() => 'sentryCloudflareVitePlugin'),
}));

const baseConfigHookObject = vi.hoisted(() => ({
logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn() },
injectScript: vi.fn(),
updateConfig: vi.fn(),
}));

describe('Cloudflare Pages vs Workers detection', () => {
beforeEach(() => {
vi.clearAllMocks();
getWranglerConfig.mockReturnValue(null);
});

describe('Cloudflare Workers (no pages_build_output_dir)', () => {
it('adds Cloudflare Vite plugins for Workers production build', async () => {
getWranglerConfig.mockReturnValue({
filename: 'wrangler.json',
content: JSON.stringify({
main: 'dist/_worker.js/index.js',
assets: { directory: './dist' },
}),
});

const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/cloudflare' },
},
command: 'build',
});

expect(baseConfigHookObject.updateConfig).toHaveBeenCalledWith(
expect.objectContaining({
vite: expect.objectContaining({
plugins: expect.arrayContaining(['sentryCloudflareNodeWarningPlugin', 'sentryCloudflareVitePlugin']),
}),
}),
);
});

it('adds Cloudflare Vite plugins when no wrangler config exists', async () => {
getWranglerConfig.mockReturnValue(null);

const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/cloudflare' },
},
command: 'build',
});

expect(baseConfigHookObject.updateConfig).toHaveBeenCalledWith(
expect.objectContaining({
vite: expect.objectContaining({
plugins: expect.arrayContaining(['sentryCloudflareNodeWarningPlugin', 'sentryCloudflareVitePlugin']),
}),
}),
);
});
});

describe('Cloudflare Pages (with pages_build_output_dir)', () => {
it('does not show warning for Pages project with wrangler.json', async () => {
getWranglerConfig.mockReturnValue({
filename: 'wrangler.json',
content: JSON.stringify({
pages_build_output_dir: './dist',
}),
});

const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/cloudflare' },
},
command: 'build',
});

expect(baseConfigHookObject.logger.error).not.toHaveBeenCalled();
});

it('does not show warning for Pages project with wrangler.jsonc', async () => {
getWranglerConfig.mockReturnValue({
filename: 'wrangler.jsonc',
content: `{
// This is a comment
"pages_build_output_dir": "./dist"
}`,
});

const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/cloudflare' },
},
command: 'build',
});

expect(baseConfigHookObject.logger.error).not.toHaveBeenCalled();
});

it('does not show warning for Pages project with wrangler.toml', async () => {
getWranglerConfig.mockReturnValue({
filename: 'wrangler.toml',
content: `
name = "my-astro-app"
pages_build_output_dir = "./dist"
`,
});

const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/cloudflare' },
},
command: 'build',
});

expect(baseConfigHookObject.logger.error).not.toHaveBeenCalled();
});

it('does not add Cloudflare Vite plugins for Pages production build', async () => {
getWranglerConfig.mockReturnValue({
filename: 'wrangler.json',
content: JSON.stringify({
pages_build_output_dir: './dist',
}),
});

const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/cloudflare' },
},
command: 'build',
});

// Check that sentryCloudflareVitePlugin is NOT in any of the calls
expect(baseConfigHookObject.updateConfig).toHaveBeenCalledWith(
{ vite: expect.objectContaining({ plugins: ['sentryCloudflareNodeWarningPlugin'] }) },
);
});

it('still adds SSR noExternal config for Pages in dev mode', async () => {
getWranglerConfig.mockReturnValue({
filename: 'wrangler.json',
content: JSON.stringify({
pages_build_output_dir: './dist',
}),
});

const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/cloudflare' },
},
command: 'dev',
});

expect(baseConfigHookObject.updateConfig).toHaveBeenCalledWith(
expect.objectContaining({
vite: expect.objectContaining({
ssr: expect.objectContaining({
noExternal: ['@sentry/astro', '@sentry/node'],
}),
}),
}),
);
});
});

describe('Non-Cloudflare adapters', () => {
it('does not show Cloudflare warning for other adapters', async () => {
const integration = sentryAstro({});

// @ts-expect-error - the hook exists and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
config: {
// @ts-expect-error - we only need to pass what we actually use
adapter: { name: '@astrojs/vercel' },
},
command: 'build',
});

expect(baseConfigHookObject.logger.error).not.toHaveBeenCalled();
});
});
});
Loading