From 377effd740bc6f4c827062f056594a39d21ceae0 Mon Sep 17 00:00:00 2001 From: Hugo Richard Date: Fri, 7 Aug 2026 21:27:20 +0100 Subject: [PATCH 1/2] fix(fs): detect an unwritable directory from the write, not from mkdir --- .changeset/fs-drain-probe-the-write.md | 9 ++++ .changeset/fs-drain-readonly-directory.md | 2 +- packages/evlog/src/adapters/fs.ts | 44 +++++++++-------- packages/evlog/test/adapters/fs.test.ts | 57 ++++++++++++++++++++--- 4 files changed, 81 insertions(+), 31 deletions(-) create mode 100644 .changeset/fs-drain-probe-the-write.md diff --git a/.changeset/fs-drain-probe-the-write.md b/.changeset/fs-drain-probe-the-write.md new file mode 100644 index 00000000..924ad3a0 --- /dev/null +++ b/.changeset/fs-drain-probe-the-write.md @@ -0,0 +1,9 @@ +--- +"evlog": patch +--- + +The file system drain now detects an unwritable directory reliably. + +The previous check probed with `mkdir({ recursive: true })`, which is a no-op on a directory that already exists and therefore succeeds even when that directory is read-only. A deployment whose log directory was baked in still threw on every batch. The probe also ran per call rather than per resolved state, so concurrent batches could each warn. + +The write itself is now the check: the first append that fails with `EROFS`, `EACCES` or `EPERM` disables the drain for that directory and warns once. Any other failure, including a full disk, is reported by the drain as before. diff --git a/.changeset/fs-drain-readonly-directory.md b/.changeset/fs-drain-readonly-directory.md index 8649c576..dbfbe8bf 100644 --- a/.changeset/fs-drain-readonly-directory.md +++ b/.changeset/fs-drain-readonly-directory.md @@ -11,4 +11,4 @@ The file system drain disables itself when its directory is not writable. const drain = process.env.VERCEL !== '1' ? createFsDrain() : undefined ``` -The drain now probes the directory once, and on `EROFS`, `EACCES` or `EPERM` warns a single time and stops, the same way it already bows out of the Edge runtime. Attach it unconditionally. Any other failure — a full disk, a genuine bug — still propagates. +The drain now probes the directory once, and on `EROFS`, `EACCES` or `EPERM` warns a single time and stops, the same way it already disables itself in the Edge runtime. Attach it unconditionally. Any other failure — a full disk, a genuine bug — still propagates. diff --git a/packages/evlog/src/adapters/fs.ts b/packages/evlog/src/adapters/fs.ts index 0459485c..134bdc5d 100644 --- a/packages/evlog/src/adapters/fs.ts +++ b/packages/evlog/src/adapters/fs.ts @@ -26,7 +26,7 @@ const FS_FIELDS: ConfigField[] = [ ] const gitignoreWritten = new Set() -const writableDirs = new Map() +const unwritableDirs = new Set() let warnedFsEdgeRuntime = false function isEdgeRuntime(): boolean { @@ -36,7 +36,7 @@ function isEdgeRuntime(): boolean { function warnFsEdgeRuntimeOnce(): void { if (warnedFsEdgeRuntime) return warnedFsEdgeRuntime = true - console.warn('[evlog/fs] File system drain is not available on the Edge runtime. Use evlog/memory or a HTTP adapter instead.') + console.warn('[evlog/fs] File system drain is not available on the Edge runtime. Use evlog/memory or an HTTP adapter instead.') } /** Read-only or permission-denied, as opposed to a full disk or a real bug. */ @@ -46,27 +46,15 @@ function isUnwritableError(error: unknown): boolean { } /** - * Whether `dir` can be created and written to, probed once per directory. + * Record that `dir` cannot be written to, and say so once. * - * Serverless hosts mount everything outside the temp directory read-only, so a - * drain attached there would otherwise throw on every batch for the lifetime of - * the deployment. Anything other than a permission failure still propagates. + * Synchronous on purpose: concurrent batches that all fail their first write + * reach this together, and the check-then-add pair cannot interleave. */ -async function isDirWritable(dir: string): Promise { - const cached = writableDirs.get(dir) - if (cached !== undefined) return cached - - try { - await mkdir(dir, { recursive: true }) - } catch (error) { - if (!isUnwritableError(error)) throw error - writableDirs.set(dir, false) - console.warn(`[evlog/fs] "${dir}" is not writable, so the file system drain is disabled. This is expected on a serverless host, where only the temp directory is writable and does not outlive the instance — send events to a HTTP adapter instead, or set the drain's \`dir\` to a writable path.`) - return false - } - - writableDirs.set(dir, true) - return true +function markUnwritable(dir: string): void { + if (unwritableDirs.has(dir)) return + unwritableDirs.add(dir) + console.warn(`[evlog/fs] "${dir}" is not writable, so the file system drain is disabled. This is expected on a serverless host, where only the temp directory is writable and does not outlive the instance — send events to an HTTP adapter instead, or set the drain's \`dir\` to a writable path.`) } async function ensureGitignore(dir: string): Promise { @@ -187,7 +175,7 @@ export function createFsDrain(overrides?: Partial) { } const resolved = await resolveAdapterConfig('fs', FS_FIELDS, overrides) const dir = resolved.dir ?? '.evlog/logs' - if (!await isDirWritable(dir)) return null + if (unwritableDirs.has(dir)) return null return { dir, pretty: resolved.pretty ?? false, @@ -195,7 +183,17 @@ export function createFsDrain(overrides?: Partial) { maxSizePerFile: resolved.maxSizePerFile, } }, - send: writeBatchToFs, + // The write itself is the probe. `mkdir` is a no-op on a directory that + // already exists, so it succeeds on a read-only one and proves nothing; + // only the append tells you whether this host will take the events. + send: async (events, config) => { + try { + await writeBatchToFs(events, config) + } catch (error) { + if (!isUnwritableError(error)) throw error + markUnwritable(config.dir) + } + }, }) } diff --git a/packages/evlog/test/adapters/fs.test.ts b/packages/evlog/test/adapters/fs.test.ts index 163a5cca..5755d884 100644 --- a/packages/evlog/test/adapters/fs.test.ts +++ b/packages/evlog/test/adapters/fs.test.ts @@ -339,11 +339,10 @@ describe('fs adapter', () => { it('warns once and disables itself when the directory is read-only', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const readOnly = Object.assign(new Error('EROFS'), { code: 'EROFS' }) try { vi.resetModules() - mockedMkdir.mockRejectedValue(readOnly) + mockedAppendFile.mockRejectedValue(Object.assign(new Error('EROFS'), { code: 'EROFS' })) const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs') const drain = createFsDrainFresh({ dir: '/var/task/.evlog/logs' }) @@ -352,9 +351,51 @@ describe('fs adapter', () => { expect(warnSpy).toHaveBeenCalledTimes(1) expect(warnSpy.mock.calls[0]?.[0]).toContain('not writable') - expect(mockedAppendFile).not.toHaveBeenCalled() - // Probed once, then answered from cache. - expect(mockedMkdir).toHaveBeenCalledTimes(1) + // The first batch attempts the write; the second never reaches it. + expect(mockedAppendFile).toHaveBeenCalledTimes(1) + } finally { + warnSpy.mockRestore() + } + }) + + it('disables itself when the directory exists but rejects the write', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + try { + vi.resetModules() + // `mkdir` is a no-op on an existing directory, so it succeeds even when + // that directory is read-only. Only the append reveals it. + mockedMkdir.mockResolvedValue(undefined) + mockedAppendFile.mockRejectedValue(Object.assign(new Error('EACCES'), { code: 'EACCES' })) + const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs') + const drain = createFsDrainFresh({ dir: '/var/task/.evlog/logs' }) + + await drain(createDrainContext()) + await drain(createDrainContext()) + + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(mockedAppendFile).toHaveBeenCalledTimes(1) + } finally { + warnSpy.mockRestore() + } + }) + + it('warns once when concurrent batches all fail their first write', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + try { + vi.resetModules() + mockedAppendFile.mockRejectedValue(Object.assign(new Error('EROFS'), { code: 'EROFS' })) + const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs') + const drain = createFsDrainFresh({ dir: '/var/task/.evlog/logs' }) + + await Promise.all([ + drain(createDrainContext({ action: 'a' })), + drain(createDrainContext({ action: 'b' })), + drain(createDrainContext({ action: 'c' })), + ]) + + expect(warnSpy).toHaveBeenCalledTimes(1) } finally { warnSpy.mockRestore() } @@ -362,13 +403,15 @@ describe('fs adapter', () => { it('propagates a write failure that is not a permission problem', async () => { vi.resetModules() - mockedMkdir.mockRejectedValue(Object.assign(new Error('ENOSPC'), { code: 'ENOSPC' })) + mockedAppendFile.mockRejectedValue(Object.assign(new Error('ENOSPC'), { code: 'ENOSPC' })) const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs') const drain = createFsDrainFresh({ dir: '.evlog/logs' }) const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) try { - await expect(drain(createDrainContext())).rejects.toThrow('ENOSPC') + await drain(createDrainContext()) + expect(errorSpy).toHaveBeenCalled() + expect(String(errorSpy.mock.calls[0]?.[1])).toContain('ENOSPC') } finally { errorSpy.mockRestore() } From 0ac99e65534db2209960f2806880784f30733c12 Mon Sep 17 00:00:00 2001 From: Hugo Richard Date: Fri, 7 Aug 2026 21:32:34 +0100 Subject: [PATCH 2/2] docs(fs): document the disable behavior and correct the release notes --- .changeset/fs-drain-probe-the-write.md | 4 ++-- .changeset/fs-drain-readonly-directory.md | 2 +- packages/evlog/src/adapters/fs.ts | 6 ++++++ packages/evlog/test/adapters/fs.test.ts | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.changeset/fs-drain-probe-the-write.md b/.changeset/fs-drain-probe-the-write.md index 924ad3a0..9a861071 100644 --- a/.changeset/fs-drain-probe-the-write.md +++ b/.changeset/fs-drain-probe-the-write.md @@ -4,6 +4,6 @@ The file system drain now detects an unwritable directory reliably. -The previous check probed with `mkdir({ recursive: true })`, which is a no-op on a directory that already exists and therefore succeeds even when that directory is read-only. A deployment whose log directory was baked in still threw on every batch. The probe also ran per call rather than per resolved state, so concurrent batches could each warn. +The previous check probed with `mkdir({ recursive: true })`, which is a no-op on a directory that already exists and therefore succeeds even when that directory is read-only. A deployment whose log directory already existed still threw on every batch. The probe also ran per call rather than per resolved state, so concurrent batches could each warn. -The write itself is now the check: the first append that fails with `EROFS`, `EACCES` or `EPERM` disables the drain for that directory and warns once. Any other failure, including a full disk, is reported by the drain as before. +The write itself is now the check: once an append fails with `EROFS`, `EACCES` or `EPERM`, the drain is disabled for that directory and warns once. Batches already in flight when that happens still attempt their own append. Any other failure, including a full disk, is reported by the drain as before. diff --git a/.changeset/fs-drain-readonly-directory.md b/.changeset/fs-drain-readonly-directory.md index dbfbe8bf..4096b198 100644 --- a/.changeset/fs-drain-readonly-directory.md +++ b/.changeset/fs-drain-readonly-directory.md @@ -11,4 +11,4 @@ The file system drain disables itself when its directory is not writable. const drain = process.env.VERCEL !== '1' ? createFsDrain() : undefined ``` -The drain now probes the directory once, and on `EROFS`, `EACCES` or `EPERM` warns a single time and stops, the same way it already disables itself in the Edge runtime. Attach it unconditionally. Any other failure — a full disk, a genuine bug — still propagates. +The drain now disables itself once it observes an `EROFS`, `EACCES` or `EPERM` write failure for that directory, warning a single time, the same way it already disables itself in the Edge runtime. Attach it unconditionally. Any other failure — a full disk, a genuine bug — still propagates. diff --git a/packages/evlog/src/adapters/fs.ts b/packages/evlog/src/adapters/fs.ts index 134bdc5d..cab80673 100644 --- a/packages/evlog/src/adapters/fs.ts +++ b/packages/evlog/src/adapters/fs.ts @@ -164,6 +164,12 @@ export async function writeBatchToFs(events: WideEvent[], config: FsConfig): Pro * pretty: true, * })) * ``` + * + * @remarks + * A write that fails with `EROFS`, `EACCES` or `EPERM` marks the configured + * directory unavailable and disables the drain for the rest of the process, + * after warning once. Attaching it on a host without a writable directory is + * therefore safe, though the events go nowhere. */ export function createFsDrain(overrides?: Partial) { return defineDrain({ diff --git a/packages/evlog/test/adapters/fs.test.ts b/packages/evlog/test/adapters/fs.test.ts index 5755d884..1f222f90 100644 --- a/packages/evlog/test/adapters/fs.test.ts +++ b/packages/evlog/test/adapters/fs.test.ts @@ -401,7 +401,7 @@ describe('fs adapter', () => { } }) - it('propagates a write failure that is not a permission problem', async () => { + it('reports a write failure that is not a permission problem', async () => { vi.resetModules() mockedAppendFile.mockRejectedValue(Object.assign(new Error('ENOSPC'), { code: 'ENOSPC' })) const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs')