Skip to content

Undefined filename in require #168

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.

Sign up for GitHub

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

Open
cursor wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Undefined filename in require #168

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 20 additions & 14 deletions src/utils/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
* It's used by virtually all other modules for status reporting and error logging.
*/

import { createRequire } from 'node:module';
import { resolve } from 'node:path';
// Note: Removed "import * as Sentry from '@sentry/node'" to prevent native module loading at import time

const SENTRY_ENABLED =
Expand Down Expand Up @@ -58,16 +56,17 @@ function isTestEnv(): boolean {

type SentryModule = typeof import('@sentry/node');

const require = createRequire(
typeof __filename === 'string' ? __filename : resolve(process.cwd(), 'package.json'),
);
let cachedSentry: SentryModule | null = null;
let sentryLoadAttempted = false;

function loadSentrySync(): SentryModule | null {
async function loadSentryAsync(): Promise<SentryModule | null> {
if (!SENTRY_ENABLED || isTestEnv()) return null;
if (cachedSentry) return cachedSentry;
if (sentryLoadAttempted) return null;

sentryLoadAttempted = true;
try {
cachedSentry = require('@sentry/node') as SentryModule;
Copy link
Contributor Author

@cursor cursor bot Jan 7, 2026

Choose a reason for hiding this comment

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

Early calls during Sentry loading silently dropped

Medium Severity

The loadSentryAsync function sets sentryLoadAttempted = true before the async import() completes. Any log calls that occur while the first import is still in progress will find sentryLoadAttempted is true but cachedSentry is still null, causing the function to return null immediately. This means error messages logged during the Sentry loading window are silently lost and never sent to Sentry. The loading promise could be stored and returned to subsequent callers to ensure all callbacks execute once loading completes.

Fix in Cursor Fix in Web

Copy link
Owner

@cameroncooke cameroncooke Jan 7, 2026

Choose a reason for hiding this comment

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

@cursor fix

Copy link
Contributor Author

@cursor cursor bot Jan 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues.

Please try again later or Open in Web to view the status.

Open in Cursor Open in Web

Learn more about Cursor Agents

Copy link
Owner

@cameroncooke cameroncooke Jan 8, 2026

Choose a reason for hiding this comment

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

@cursor fix

Copy link
Contributor Author

@cursor cursor bot Jan 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues.

Please try again later or Open in Web to view the status.

Open in Cursor Open in Web

Learn more about Cursor Agents

cachedSentry = await import('@sentry/node');
return cachedSentry;
} catch {
// If @sentry/node is not installed in some environments, fail silently.
Comment on lines 63 to 72
Copy link

@sentry sentry bot Jan 7, 2026

Choose a reason for hiding this comment

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

Bug: The switch to asynchronous Sentry loading in withSentry creates a race condition, causing error logs to be lost if the process exits before the async operation completes.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The withSentry function was changed to load Sentry asynchronously using loadSentryAsync().then(...). This defers the execution of the callback, which sends the error report, to a future microtask. In scenarios where the application exits shortly after logging an error, such as in fatal error handlers that call process.exit(), the process will terminate before the microtask queue is processed. This will cause the error log to be lost and never sent to Sentry, defeating the purpose of the error reporting mechanism, especially for unhandled exceptions where immediate process termination is common.

💡 Suggested Fix

Revert withSentry to its previous synchronous implementation. Since @sentry/node is already loaded synchronously in other parts of the application, the asynchronous loading is unnecessary. If async loading must be kept, error handlers should be modified to await the logging operation's completion before exiting the process.

🤖 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/utils/logger.ts#L63-L72

Potential issue: The `withSentry` function was changed to load Sentry asynchronously
using `loadSentryAsync().then(...)`. This defers the execution of the callback, which
sends the error report, to a future microtask. In scenarios where the application exits
shortly after logging an error, such as in fatal error handlers that call
`process.exit()`, the process will terminate before the microtask queue is processed.
This will cause the error log to be lost and never sent to Sentry, defeating the purpose
of the error reporting mechanism, especially for unhandled exceptions where immediate
process termination is common.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 530353

Expand All @@ -76,13 +75,20 @@ function loadSentrySync(): SentryModule | null {
}

function withSentry(cb: (s: SentryModule) => void): void {
const s = loadSentrySync();
if (!s) return;
try {
cb(s);
} catch {
// no-op: avoid throwing inside logger
}
// Fire-and-forget async Sentry loading
loadSentryAsync()
.then((s) => {
if (s) {
try {
cb(s);
} catch {
// no-op: avoid throwing inside logger
}
}
})
.catch(() => {
// no-op: avoid throwing inside logger
});
}

if (!SENTRY_ENABLED) {
Expand Down
Loading