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
5 changes: 5 additions & 0 deletions .changeset/fix-v3-h3-optional-peer-leak.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"evlog": patch
---

Fix `evlog/nitro/v3` pulling in the optional `h3` peer. The v3 plugin shared a deferred-drain helper from the v2 module, which imports `getHeaders` from `h3`, so the v3 bundle referenced `h3` even though the v3 runtime never uses it. Consumers that don't install `h3` directly (e.g. Nitro v3 / TanStack Start on Vite) failed to build with `"getHeaders" is not exported by "__vite-optional-peer-dep:h3:evlog"`. The helper now lives in an h3-free module, so the v3 path no longer references `h3`.
2 changes: 1 addition & 1 deletion packages/evlog/src/nitro-v3/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { readCodeSnippetFromDisk } from '../shared/pretty-error-snippet.node'
import { enrichErrorStackForDev } from '../shared/enrich-error-stack.node'
import { shouldLog, getServiceForPath, extractErrorStatus } from '../nitro'
import { extendDeferredDrain } from '../nitro/enrich-drain'
import { extendDeferredDrain } from '../nitro/deferred-drain'
import { normalizeRedactConfig } from '../redact'
import { resolveEvlogConfigForNitroPlugin, setActiveNitroRuntime } from '../shared/nitroConfigBridge'
import { bindStreamingResponseLifecycle, shouldDeferEmitForResponse } from '../shared/streamResponse'
Expand Down Expand Up @@ -71,7 +71,7 @@
}
}

async function callDrainHook(

Check warning on line 74 in packages/evlog/src/nitro-v3/plugin.ts

View workflow job for this annotation

GitHub Actions / autofix

Async function 'callDrainHook' has too many parameters (5). Maximum allowed is 4

Check warning on line 74 in packages/evlog/src/nitro-v3/plugin.ts

View workflow job for this annotation

GitHub Actions / lint

Async function 'callDrainHook' has too many parameters (5). Maximum allowed is 4
hooks: Hooks,
emittedEvent: WideEvent | null,
event: HTTPEvent,
Expand Down Expand Up @@ -124,7 +124,7 @@
}
}

async function callEnrichAndDrain(

Check warning on line 127 in packages/evlog/src/nitro-v3/plugin.ts

View workflow job for this annotation

GitHub Actions / autofix

Async function 'callEnrichAndDrain' has too many parameters (5). Maximum allowed is 4

Check warning on line 127 in packages/evlog/src/nitro-v3/plugin.ts

View workflow job for this annotation

GitHub Actions / lint

Async function 'callEnrichAndDrain' has too many parameters (5). Maximum allowed is 4
hooks: Hooks,
emittedEvent: WideEvent | null,
event: HTTPEvent,
Expand Down
12 changes: 12 additions & 0 deletions packages/evlog/src/nitro/deferred-drain.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/** @internal Extend drain lifetime on Cloudflare without blocking Nitro Node responses. */
export function extendDeferredDrain(
drainPromise: Promise<unknown>,
waitUntil?: (promise: Promise<unknown>) => void,
): void {
void drainPromise.catch((err) => {
console.error('[evlog] background drain failed:', err)
})
if (typeof waitUntil === 'function') {
waitUntil(drainPromise)
}
}
14 changes: 1 addition & 13 deletions packages/evlog/src/nitro/enrich-drain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getHeaders } from 'h3'
import { getGlobalPluginRunner } from '../logger'
import type { EnrichContext, ServerEvent, WideEvent } from '../types'
import { filterSafeHeaders } from '../utils'
import { extendDeferredDrain } from './deferred-drain'

function getSafeHeaders(event: ServerEvent): Record<string, string> {
const allHeaders = getHeaders(event as Parameters<typeof getHeaders>[0])
Expand Down Expand Up @@ -55,19 +56,6 @@ function buildHookContext(event: ServerEvent): Omit<EnrichContext, 'event'> {
}
}

/** @internal Extend drain lifetime on Cloudflare without blocking Nitro Node responses. */
export function extendDeferredDrain(
drainPromise: Promise<unknown>,
waitUntil?: (promise: Promise<unknown>) => void,
): void {
void drainPromise.catch((err) => {
console.error('[evlog] background drain failed:', err)
})
if (typeof waitUntil === 'function') {
waitUntil(drainPromise)
}
}

function resolveDeferredWaitUntil(event: ServerEvent): ((promise: Promise<unknown>) => void) | undefined {
if (globalThis.navigator?.userAgent !== 'Cloudflare-Workers') return undefined
const waitUntilCtx = event.context.cloudflare?.context ?? event.context
Expand Down
64 changes: 64 additions & 0 deletions packages/evlog/test/nitro-v3/h3-peer-isolation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { existsSync, readFileSync } from 'node:fs'
import { dirname, resolve } from 'node:path'
import { describe, expect, it } from 'vitest'

const v3Plugin = resolve(__dirname, '../../dist/nitro/v3/plugin.mjs')
const distExists = existsSync(v3Plugin)

if (!distExists) {
console.warn('[evlog test] Skipping h3 peer isolation: dist/ not found. Run `pnpm --filter evlog run build` first.')
}

// h3 is legitimately imported by the v2 runtime chunks, so it can't be forbidden
// dist-wide. Collect the chunks the v3 plugin actually reaches (it pulls in
// shared root-level chunks via `../../*.mjs`) and forbid h3 only within that set.
function collectV3Chunks(entry: string): string[] {
const seen = new Set<string>()
const relativeRe = /(?:import|export)\s+(?:[^'"]*?\sfrom\s+)?['"](\.[^'"]+)['"]|import\s*\(\s*['"](\.[^'"]+)['"]\s*\)/g
Comment thread
jmcgoldrick marked this conversation as resolved.

const walk = (file: string): void => {
if (seen.has(file)) return
seen.add(file)
let source: string
try {
source = readFileSync(file, 'utf8')
} catch {
return
}
let match: RegExpExecArray | null
while ((match = relativeRe.exec(source))) {
walk(resolve(dirname(file), match[1] ?? match[2]))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚑ Quick win

Add .mjs extension when resolving relative imports.

Built chunks use the .mjs extension, but line 31 resolves bare paths like ./foo without appending .mjs. The subsequent readFileSync on line 24 will fail (silently caught on line 26), causing the walk to miss reachable chunks that might import h3.

πŸ› Proposed fix: append `.mjs` if no extension present
     let match: RegExpExecArray | null
     while ((match = relativeRe.exec(source))) {
-      walk(resolve(dirname(file), match[1] ?? match[2]))
+      const relPath = match[1] ?? match[2]
+      const absPath = resolve(dirname(file), relPath)
+      walk(absPath.endsWith('.mjs') ? absPath : `${absPath}.mjs`)
     }
   }
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/evlog/test/nitro-v3/h3-peer-isolation.test.ts` at line 31, The
resolver call inside the import-walking logic (the walk(resolve(dirname(file),
match[1] ?? match[2])) invocation) doesn't add an extension for relative
imports, so change the resolution to detect if the matched path has no extension
(use path.extname on match[1] ?? match[2]) and, if missing, append ".mjs" before
calling walk; update the same computed resolved path used for readFileSync to
ensure files built as .mjs are found (keep using dirname(file) and the match
variable names to locate the change).

}
}
Comment thread
jmcgoldrick marked this conversation as resolved.

walk(entry)
return [...seen]
}

/**
* Regression: the v3 plugin reused `extendDeferredDrain` from `nitro/enrich-drain`,
* which imports `getHeaders` from the optional `h3` peer for the v2 runtime. v3
* consumers without a direct `h3` dependency hit the stubbed optional peer and
* failed to build with `"getHeaders" is not exported by "__vite-optional-peer-dep:h3:evlog"`.
* No chunk in the v3 plugin's graph may import `h3`.
*/
describe.skipIf(!distExists)('evlog/nitro/v3 avoids the optional h3 peer', () => {
it('no chunk reachable from the v3 plugin imports h3', () => {
const chunks = collectV3Chunks(v3Plugin)
expect(chunks.length).toBeGreaterThan(0)

const forbidden = [
'from "h3"',
'from \'h3\'',
'import("h3")',
'import(\'h3\')',
]

for (const file of chunks) {
const src = readFileSync(file, 'utf8')
for (const needle of forbidden) {
expect(src, file).not.toContain(needle)
}
}
Comment thread
jmcgoldrick marked this conversation as resolved.
})
})
Loading