Skip to content
Merged
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
28 changes: 28 additions & 0 deletions dev-packages/e2e-tests/test-applications/effect-browser/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# dependencies
/node_modules
/.pnp
.pnp.js

# testing
/coverage

# production
/build
/dist

# misc
.DS_Store
.env.local
.env.development.local
.env.test.local
.env.production.local

npm-debug.log*
yarn-debug.log*
yarn-error.log*

/test-results/
/playwright-report/
/playwright/.cache/

!*.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@sentry:registry=http://127.0.0.1:4873
@sentry-internal:registry=http://127.0.0.1:4873
52 changes: 52 additions & 0 deletions dev-packages/e2e-tests/test-applications/effect-browser/build.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import * as path from 'path';
import * as url from 'url';
import HtmlWebpackPlugin from 'html-webpack-plugin';
import TerserPlugin from 'terser-webpack-plugin';
import webpack from 'webpack';

const __dirname = path.dirname(url.fileURLToPath(import.meta.url));

webpack(
{
entry: path.join(__dirname, 'src/index.js'),
output: {
path: path.join(__dirname, 'build'),
filename: 'app.js',
},
optimization: {
minimize: true,
minimizer: [new TerserPlugin()],
},
plugins: [
new webpack.EnvironmentPlugin(['E2E_TEST_DSN']),
new HtmlWebpackPlugin({
template: path.join(__dirname, 'public/index.html'),
}),
],
performance: {
hints: false,
},
mode: 'production',
},
(err, stats) => {
if (err) {
console.error(err.stack || err);
if (err.details) {
console.error(err.details);
}
return;
}
Copy link

Choose a reason for hiding this comment

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

Build script silently swallows webpack fatal errors

Medium Severity

When webpack invokes the callback with a fatal err (e.g. configuration error or file system failure — distinct from compilation errors), the handler logs the error and calls return without process.exit(1). The build process then exits with code 0, making CI falsely report a successful build even though no output was produced, causing the subsequent pnpm start to serve stale or missing files.

Fix in Cursor Fix in Web

Comment on lines +32 to +38
Copy link

Choose a reason for hiding this comment

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

Bug: The webpack build script logs fatal configuration errors but doesn't exit with a failure code, causing the CI build to appear successful despite the failure.
Severity: MEDIUM

Suggested Fix

Add process.exit(1) within the if (err) block after logging the error. This will ensure that any fatal webpack configuration error causes the build process to fail with a non-zero exit code, correctly signaling a build failure in the CI pipeline.

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: dev-packages/e2e-tests/test-applications/effect-browser/build.mjs#L32-L38

Potential issue: In the webpack build script, if a fatal configuration error occurs, the
`err` object is logged, but the process does not exit with a non-zero status code.
Instead, it returns and exits cleanly with code 0. This causes the CI/CD pipeline to
incorrectly report the build as successful. Subsequent steps, like starting the
application server, will then fail with a confusing error because the build artifacts
were never created. This masks the true root cause of the failure, which is the webpack
configuration error.

Did we get this right? 👍 / 👎 to inform future reviews.


const info = stats.toJson();

if (stats.hasErrors()) {
console.error(info.errors);
process.exit(1);
}

if (stats.hasWarnings()) {
console.warn(info.warnings);
process.exit(1);
}
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"name": "effect-browser-test-app",
"version": "1.0.0",
"private": true,
"scripts": {
"start": "serve -s build",
"build": "node build.mjs",
"test": "playwright test",
"clean": "npx rimraf node_modules pnpm-lock.yaml",
"test:build": "pnpm install && pnpm build",
"test:assert": "pnpm test"
},
"dependencies": {
"@sentry/effect": "latest || *",
"@types/node": "^18.19.1",
"effect": "^3.19.19",
"typescript": "~5.0.0"
},
"devDependencies": {
"@playwright/test": "~1.56.0",
"@sentry-internal/test-utils": "link:../../../test-utils",
"webpack": "^5.91.0",
"serve": "14.0.1",
"terser-webpack-plugin": "^5.3.10",
"html-webpack-plugin": "^5.6.0"
},
"browserslist": {
"production": [
">0.2%",
"not dead",
"not op_mini all"
],
"development": [
"last 1 chrome version",
"last 1 firefox version",
"last 1 safari version"
]
},
"volta": {
"extends": "../../package.json"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { getPlaywrightConfig } from '@sentry-internal/test-utils';

const config = getPlaywrightConfig({
startCommand: `pnpm start`,
});

export default config;
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Effect Browser App</title>
</head>
<body>
<h1>Effect Browser E2E Test</h1>

<div id="app">
<section>
<h2>Error Tests</h2>
<input type="button" value="Capture Exception" id="exception-button" />
</section>

<section>
<h2>Effect Span Tests</h2>
<input type="button" value="Create Effect Span" id="effect-span-button" />
<span id="effect-span-result"></span>
</section>

<section>
<h2>Effect Failure Tests</h2>
<input type="button" value="Effect.fail()" id="effect-fail-button" />
<span id="effect-fail-result"></span>
<br />
<input type="button" value="Effect.die()" id="effect-die-button" />
<span id="effect-die-result"></span>
</section>

<section>
<h2>Log Tests</h2>
<input type="button" value="Send Logs" id="log-button" />
<span id="log-result"></span>
<br />
<input type="button" value="Send Log with Context" id="log-context-button" />
<span id="log-context-result"></span>
</section>

<section id="navigation">
<h2>Navigation Test</h2>
<a id="navigation-link" href="#navigation-target">Navigation Link</a>
<div id="navigation-target" style="margin-top: 50px">Navigated Element</div>
</section>
</div>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// @ts-check
import * as Sentry from '@sentry/effect';
import { Cause, Effect, Layer, Logger, LogLevel, Runtime } from 'effect';

const LogLevelLive = Logger.minimumLogLevel(LogLevel.Debug);
const AppLayer = Layer.mergeAll(
Sentry.effectLayer({
dsn: process.env.E2E_TEST_DSN,
integrations: [
Sentry.browserTracingIntegration({
_experiments: { enableInteractions: true },
}),
],
tracesSampleRate: 1.0,
release: 'e2e-test',
environment: 'qa',
tunnel: 'http://localhost:3031',
enableLogs: true,
enableEffectLogs: true,
}),
LogLevelLive,
);

const runtime = Layer.toRuntime(AppLayer).pipe(Effect.scoped, Effect.runSync);

const runEffect = fn => Runtime.runPromise(runtime)(fn());

document.getElementById('exception-button')?.addEventListener('click', () => {
throw new Error('I am an error!');
});

document.getElementById('effect-span-button')?.addEventListener('click', async () => {
await runEffect(() =>
Effect.gen(function* () {
yield* Effect.sleep('50 millis');
yield* Effect.sleep('25 millis').pipe(Effect.withSpan('nested-span'));
}).pipe(Effect.withSpan('custom-effect-span', { kind: 'internal' })),
);
const el = document.getElementById('effect-span-result');
if (el) el.textContent = 'Span sent!';
});

document.getElementById('effect-fail-button')?.addEventListener('click', async () => {
try {
await runEffect(() => Effect.fail(new Error('Effect failure')));
} catch {
const el = document.getElementById('effect-fail-result');
if (el) el.textContent = 'Effect failed (expected)';
}
});

document.getElementById('effect-die-button')?.addEventListener('click', async () => {
try {
await runEffect(() => Effect.die('Effect defect'));
} catch {
const el = document.getElementById('effect-die-result');
if (el) el.textContent = 'Effect died (expected)';
}
});

document.getElementById('log-button')?.addEventListener('click', async () => {
await runEffect(() =>
Effect.gen(function* () {
yield* Effect.logDebug('Debug log from Effect');
yield* Effect.logInfo('Info log from Effect');
yield* Effect.logWarning('Warning log from Effect');
yield* Effect.logError('Error log from Effect');
}),
);
const el = document.getElementById('log-result');
if (el) el.textContent = 'Logs sent!';
});

document.getElementById('log-context-button')?.addEventListener('click', async () => {
await runEffect(() =>
Effect.logInfo('Log with context').pipe(
Effect.annotateLogs('userId', '12345'),
Effect.annotateLogs('action', 'test'),
),
);
const el = document.getElementById('log-context-result');
if (el) el.textContent = 'Log with context sent!';
});

document.getElementById('navigation-link')?.addEventListener('click', () => {
document.getElementById('navigation-target')?.scrollIntoView({ behavior: 'smooth' });
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { startEventProxyServer } from '@sentry-internal/test-utils';

startEventProxyServer({
port: 3031,
proxyServerName: 'effect-browser',
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { expect, test } from '@playwright/test';
import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';

test('captures an error', async ({ page }) => {
const errorEventPromise = waitForError('effect-browser', event => {
return !event.type && event.exception?.values?.[0]?.value === 'I am an error!';
});

await page.goto('/');

const exceptionButton = page.locator('id=exception-button');
await exceptionButton.click();

const errorEvent = await errorEventPromise;

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('I am an error!');
expect(errorEvent.transaction).toBe('/');

expect(errorEvent.request).toEqual({
url: 'http://localhost:3030/',
headers: expect.any(Object),
});

expect(errorEvent.contexts?.trace).toEqual({
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
span_id: expect.stringMatching(/[a-f0-9]{16}/),
});
});

test('sets correct transactionName', async ({ page }) => {
const transactionPromise = waitForTransaction('effect-browser', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
});

const errorEventPromise = waitForError('effect-browser', event => {
return !event.type && event.exception?.values?.[0]?.value === 'I am an error!';
});

await page.goto('/');
const transactionEvent = await transactionPromise;

const exceptionButton = page.locator('id=exception-button');
await exceptionButton.click();

const errorEvent = await errorEventPromise;

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('I am an error!');
expect(errorEvent.transaction).toEqual('/');

expect(errorEvent.contexts?.trace).toEqual({
trace_id: transactionEvent.contexts?.trace?.trace_id,
span_id: expect.not.stringContaining(transactionEvent.contexts?.trace?.span_id || ''),
});
});
Loading
Loading