From dfd6b1ea723934d13afdb921cd5b7c5ffbd75d00 Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Thu, 6 Aug 2026 08:43:43 +0000 Subject: [PATCH 1/4] Fix Deno filesystem error context --- .changeset/deno-write-copy-errors.md | 5 +++ packages/platform-deno/src/DenoFileSystem.ts | 31 ++++++++++---- packages/platform-deno/src/internal/error.ts | 3 ++ .../platform-deno/test/DenoFileSystem.test.ts | 42 +++++++++++++++++-- .../platform-deno/test/internal/error.test.ts | 2 +- 5 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 .changeset/deno-write-copy-errors.md diff --git a/.changeset/deno-write-copy-errors.md b/.changeset/deno-write-copy-errors.md new file mode 100644 index 00000000000..2b93ed38e63 --- /dev/null +++ b/.changeset/deno-write-copy-errors.md @@ -0,0 +1,5 @@ +--- +"@effect/platform-deno": patch +--- + +Preserve high-level filesystem error context for `writeFile` and normalize Deno `AlreadyExists` errors from `copy`. diff --git a/packages/platform-deno/src/DenoFileSystem.ts b/packages/platform-deno/src/DenoFileSystem.ts index bf8a4188ce0..3f7c51078d5 100644 --- a/packages/platform-deno/src/DenoFileSystem.ts +++ b/packages/platform-deno/src/DenoFileSystem.ts @@ -272,13 +272,17 @@ class FileImpl implements FileSystem.File { ) } - private writeChunk(method: string, buffer: Uint8Array) { + private writeChunk( + method: string, + buffer: Uint8Array, + pathOrDescriptor?: string | number + ) { return Effect.suspend(() => { const position = this.position return Effect.map( tryPromise( method, - undefined, + pathOrDescriptor, async () => { if (!this.append && this.nativePosition !== position) { this.file.seekSync(position, Deno.SeekMode.Start) @@ -304,24 +308,33 @@ class FileImpl implements FileSystem.File { return this.writeChunk("write", buffer) } - private writeAllChunk(buffer: Uint8Array): Effect.Effect { - return Effect.flatMap(this.writeChunk("writeAll", buffer), (bytesWritten) => { + private writeAllChunk( + buffer: Uint8Array, + method: string, + pathOrDescriptor?: string | number + ): Effect.Effect { + return Effect.flatMap(this.writeChunk(method, buffer, pathOrDescriptor), (bytesWritten) => { if (bytesWritten === BigInt(0)) { return Effect.fail(PlatformError.systemError({ module: "FileSystem", - method: "writeAll", + method, + pathOrDescriptor, _tag: "WriteZero", description: "write returned 0 bytes written" })) } return bytesWritten < buffer.length - ? this.writeAllChunk(buffer.subarray(Number(bytesWritten))) + ? this.writeAllChunk(buffer.subarray(Number(bytesWritten)), method, pathOrDescriptor) : Effect.void }) } - writeAll(buffer: Uint8Array) { - return buffer.length === 0 ? Effect.void : this.writeAllChunk(buffer) + writeAll( + buffer: Uint8Array, + method = "writeAll", + pathOrDescriptor?: string | number + ) { + return buffer.length === 0 ? Effect.void : this.writeAllChunk(buffer, method, pathOrDescriptor) } } @@ -452,7 +465,7 @@ const writeFile: FileSystem.FileSystem["writeFile"] = (path, data, options) => { } return Effect.acquireUseRelease( tryPromise("writeFile", path, () => Deno.open(path, openOptions(flag, options?.mode))), - (file) => new FileImpl(file, flag.startsWith("a")).writeAll(data), + (file) => new FileImpl(file, flag.startsWith("a")).writeAll(data, "writeFile", path), (file) => close(file, "writeFile", path) ) } diff --git a/packages/platform-deno/src/internal/error.ts b/packages/platform-deno/src/internal/error.ts index 97c73ea055f..11a0f5a0a8e 100644 --- a/packages/platform-deno/src/internal/error.ts +++ b/packages/platform-deno/src/internal/error.ts @@ -16,6 +16,9 @@ export const handleError = ( let tag: SystemErrorTag = "Unknown" switch (denoError?.name) { + case "AlreadyExists": + tag = "AlreadyExists" + break case "NotCapable": tag = "PermissionDenied" break diff --git a/packages/platform-deno/test/DenoFileSystem.test.ts b/packages/platform-deno/test/DenoFileSystem.test.ts index 96843f6c246..b404f8bc23b 100644 --- a/packages/platform-deno/test/DenoFileSystem.test.ts +++ b/packages/platform-deno/test/DenoFileSystem.test.ts @@ -1,9 +1,45 @@ import * as DenoFileSystem from "@effect/platform-deno/DenoFileSystem" -import { describe } from "@effect/vitest" +import { assert, describe, it } from "@effect/vitest" +import * as Effect from "effect/Effect" +import * as FileSystem from "effect/FileSystem" +import { SystemError } from "effect/PlatformError" import { testLayer } from "../../effect/test/FileSystem.test-utils.ts" -describe("FileSystem", () => +describe("FileSystem", () => { testLayer(DenoFileSystem.layer, { accessOnDirectory: false, tempFileScopedRemovesDirectory: false - })) + }) + + it.effect("reports delegated write errors as writeFile errors", () => + Effect.gen(function*() { + const fs = yield* FileSystem.FileSystem + const root = yield* fs.makeTempDirectoryScoped() + const path = `${root}/file.txt` + yield* fs.writeFileString(path, "seed") + + const error = yield* Effect.flip(fs.writeFileString(path, "data", { flag: "r" })) + + assert(error.reason instanceof SystemError) + assert.strictEqual(error.reason.method, "writeFile") + assert.strictEqual(error.reason.pathOrDescriptor, path) + }).pipe(Effect.provide(DenoFileSystem.layer))) + + it.effect("maps an existing copy destination to AlreadyExists", () => + Effect.gen(function*() { + const fs = yield* FileSystem.FileSystem + const root = yield* fs.makeTempDirectoryScoped() + const source = `${root}/source.txt` + const destination = `${root}/destination.txt` + yield* fs.writeFileString(source, "source") + yield* fs.writeFileString(destination, "destination") + + const error = yield* Effect.flip(fs.copy(source, destination, { overwrite: false })) + + assert(error.reason instanceof SystemError) + assert.strictEqual(error.reason._tag, "AlreadyExists") + assert.strictEqual(error.reason.method, "copy") + assert.strictEqual(error.reason.pathOrDescriptor, source) + assert.strictEqual(yield* fs.readFileString(destination), "destination") + }).pipe(Effect.provide(DenoFileSystem.layer))) +}) diff --git a/packages/platform-deno/test/internal/error.test.ts b/packages/platform-deno/test/internal/error.test.ts index a31f54170e6..2605eec8dc6 100644 --- a/packages/platform-deno/test/internal/error.test.ts +++ b/packages/platform-deno/test/internal/error.test.ts @@ -9,7 +9,7 @@ describe("handleError", () => { const cases: ReadonlyArray = [ [withCode(new Deno.errors.NotFound(), "ENOENT"), "NotFound"], [withCode(new Deno.errors.NotADirectory(), "ENOTDIR"), "BadResource"], - [withCode(new Deno.errors.AlreadyExists(), "EEXIST"), "AlreadyExists"], + [new Deno.errors.AlreadyExists(), "AlreadyExists"], [withCode(new Deno.errors.IsADirectory(), "EISDIR"), "BadResource"], [withCode(new Deno.errors.PermissionDenied(), "EACCES"), "PermissionDenied"], [new Deno.errors.NotCapable(), "PermissionDenied"], From 9021806daeadc09bf44c9aed2280824add376fad Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Thu, 6 Aug 2026 09:05:09 +0000 Subject: [PATCH 2/4] Address Deno filesystem review feedback --- packages/effect/test/FileSystem.test-utils.ts | 29 ++++++++++++- packages/platform-deno/src/DenoFileSystem.ts | 38 +++++++---------- .../platform-deno/test/DenoFileSystem.test.ts | 42 ++----------------- 3 files changed, 46 insertions(+), 63 deletions(-) diff --git a/packages/effect/test/FileSystem.test-utils.ts b/packages/effect/test/FileSystem.test-utils.ts index a89dfb39ba5..1c72991f9a9 100644 --- a/packages/effect/test/FileSystem.test-utils.ts +++ b/packages/effect/test/FileSystem.test-utils.ts @@ -1,8 +1,9 @@ import { assert, expect, it } from "@effect/vitest" -import { Array } from "effect" +import { Array, Result } from "effect" import * as Effect from "effect/Effect" import * as Fs from "effect/FileSystem" import type * as Layer from "effect/Layer" +import * as PlatformError from "effect/PlatformError" import * as Stream from "effect/Stream" export interface TestLayerOptions { @@ -137,8 +138,11 @@ export const testLayer = (layer: Layer.Layer, options: Test const fs = yield* Fs.FileSystem const path = yield* fs.makeTempFile() - yield* fs.writeFileString(path, "data", { flag: "r" }).pipe(Effect.flip) + const error = yield* fs.writeFileString(path, "data", { flag: "r" }).pipe(Effect.flip) + assert(error.reason instanceof PlatformError.SystemError) + assert.strictEqual(error.reason.method, "writeFile") + assert.strictEqual(error.reason.pathOrDescriptor, path) assert.strictEqual(yield* fs.readFileString(path), "") }))) @@ -165,6 +169,27 @@ export const testLayer = (layer: Layer.Layer, options: Test assert.strictEqual(yield* fs.readFileString(path), "first") }))) + it("copy with overwrite false preserves an existing destination", () => + runPromise(Effect.gen(function*() { + const fs = yield* Fs.FileSystem + const root = yield* fs.makeTempDirectory() + const source = `${root}/source.txt` + const destination = `${root}/destination.txt` + yield* fs.writeFileString(source, "source") + yield* fs.writeFileString(destination, "destination") + + const result = yield* Effect.result(fs.copy(source, destination, { overwrite: false })) + + if (Result.isFailure(result)) { + assert(result.failure.reason instanceof PlatformError.SystemError) + assert.strictEqual(result.failure.reason._tag, "AlreadyExists") + assert.strictEqual(result.failure.reason.method, "copy") + assert.strictEqual(result.failure.reason.pathOrDescriptor, source) + } + assert.strictEqual(yield* fs.readFileString(source), "source") + assert.strictEqual(yield* fs.readFileString(destination), "destination") + }))) + it("should track the cursor position when reading", () => runPromise(Effect.gen(function*() { const fs = yield* Fs.FileSystem diff --git a/packages/platform-deno/src/DenoFileSystem.ts b/packages/platform-deno/src/DenoFileSystem.ts index 3f7c51078d5..e8d61f91058 100644 --- a/packages/platform-deno/src/DenoFileSystem.ts +++ b/packages/platform-deno/src/DenoFileSystem.ts @@ -272,17 +272,13 @@ class FileImpl implements FileSystem.File { ) } - private writeChunk( - method: string, - buffer: Uint8Array, - pathOrDescriptor?: string | number - ) { + private writeChunk(method: string, buffer: Uint8Array) { return Effect.suspend(() => { const position = this.position return Effect.map( tryPromise( method, - pathOrDescriptor, + undefined, async () => { if (!this.append && this.nativePosition !== position) { this.file.seekSync(position, Deno.SeekMode.Start) @@ -308,33 +304,24 @@ class FileImpl implements FileSystem.File { return this.writeChunk("write", buffer) } - private writeAllChunk( - buffer: Uint8Array, - method: string, - pathOrDescriptor?: string | number - ): Effect.Effect { - return Effect.flatMap(this.writeChunk(method, buffer, pathOrDescriptor), (bytesWritten) => { + private writeAllChunk(buffer: Uint8Array): Effect.Effect { + return Effect.flatMap(this.writeChunk("writeAll", buffer), (bytesWritten) => { if (bytesWritten === BigInt(0)) { return Effect.fail(PlatformError.systemError({ module: "FileSystem", - method, - pathOrDescriptor, + method: "writeAll", _tag: "WriteZero", description: "write returned 0 bytes written" })) } return bytesWritten < buffer.length - ? this.writeAllChunk(buffer.subarray(Number(bytesWritten)), method, pathOrDescriptor) + ? this.writeAllChunk(buffer.subarray(Number(bytesWritten))) : Effect.void }) } - writeAll( - buffer: Uint8Array, - method = "writeAll", - pathOrDescriptor?: string | number - ) { - return buffer.length === 0 ? Effect.void : this.writeAllChunk(buffer, method, pathOrDescriptor) + writeAll(buffer: Uint8Array) { + return buffer.length === 0 ? Effect.void : this.writeAllChunk(buffer) } } @@ -465,7 +452,14 @@ const writeFile: FileSystem.FileSystem["writeFile"] = (path, data, options) => { } return Effect.acquireUseRelease( tryPromise("writeFile", path, () => Deno.open(path, openOptions(flag, options?.mode))), - (file) => new FileImpl(file, flag.startsWith("a")).writeAll(data, "writeFile", path), + (file) => + new FileImpl(file, flag.startsWith("a")).writeAll(data).pipe( + Effect.mapError((error) => + error.reason instanceof PlatformError.SystemError + ? PlatformError.systemError({ ...error.reason, method: "writeFile", pathOrDescriptor: path }) + : error + ) + ), (file) => close(file, "writeFile", path) ) } diff --git a/packages/platform-deno/test/DenoFileSystem.test.ts b/packages/platform-deno/test/DenoFileSystem.test.ts index b404f8bc23b..96843f6c246 100644 --- a/packages/platform-deno/test/DenoFileSystem.test.ts +++ b/packages/platform-deno/test/DenoFileSystem.test.ts @@ -1,45 +1,9 @@ import * as DenoFileSystem from "@effect/platform-deno/DenoFileSystem" -import { assert, describe, it } from "@effect/vitest" -import * as Effect from "effect/Effect" -import * as FileSystem from "effect/FileSystem" -import { SystemError } from "effect/PlatformError" +import { describe } from "@effect/vitest" import { testLayer } from "../../effect/test/FileSystem.test-utils.ts" -describe("FileSystem", () => { +describe("FileSystem", () => testLayer(DenoFileSystem.layer, { accessOnDirectory: false, tempFileScopedRemovesDirectory: false - }) - - it.effect("reports delegated write errors as writeFile errors", () => - Effect.gen(function*() { - const fs = yield* FileSystem.FileSystem - const root = yield* fs.makeTempDirectoryScoped() - const path = `${root}/file.txt` - yield* fs.writeFileString(path, "seed") - - const error = yield* Effect.flip(fs.writeFileString(path, "data", { flag: "r" })) - - assert(error.reason instanceof SystemError) - assert.strictEqual(error.reason.method, "writeFile") - assert.strictEqual(error.reason.pathOrDescriptor, path) - }).pipe(Effect.provide(DenoFileSystem.layer))) - - it.effect("maps an existing copy destination to AlreadyExists", () => - Effect.gen(function*() { - const fs = yield* FileSystem.FileSystem - const root = yield* fs.makeTempDirectoryScoped() - const source = `${root}/source.txt` - const destination = `${root}/destination.txt` - yield* fs.writeFileString(source, "source") - yield* fs.writeFileString(destination, "destination") - - const error = yield* Effect.flip(fs.copy(source, destination, { overwrite: false })) - - assert(error.reason instanceof SystemError) - assert.strictEqual(error.reason._tag, "AlreadyExists") - assert.strictEqual(error.reason.method, "copy") - assert.strictEqual(error.reason.pathOrDescriptor, source) - assert.strictEqual(yield* fs.readFileString(destination), "destination") - }).pipe(Effect.provide(DenoFileSystem.layer))) -}) + })) From 40ae4e30c6b967f157c17df8833aa75e11868069 Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Thu, 6 Aug 2026 09:15:43 +0000 Subject: [PATCH 3/4] Use tags for filesystem error checks --- packages/effect/test/FileSystem.test-utils.ts | 6 ++---- packages/platform-deno/src/DenoFileSystem.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/effect/test/FileSystem.test-utils.ts b/packages/effect/test/FileSystem.test-utils.ts index 1c72991f9a9..3490d7db974 100644 --- a/packages/effect/test/FileSystem.test-utils.ts +++ b/packages/effect/test/FileSystem.test-utils.ts @@ -3,7 +3,6 @@ import { Array, Result } from "effect" import * as Effect from "effect/Effect" import * as Fs from "effect/FileSystem" import type * as Layer from "effect/Layer" -import * as PlatformError from "effect/PlatformError" import * as Stream from "effect/Stream" export interface TestLayerOptions { @@ -140,7 +139,7 @@ export const testLayer = (layer: Layer.Layer, options: Test const error = yield* fs.writeFileString(path, "data", { flag: "r" }).pipe(Effect.flip) - assert(error.reason instanceof PlatformError.SystemError) + assert(error.reason._tag !== "BadArgument") assert.strictEqual(error.reason.method, "writeFile") assert.strictEqual(error.reason.pathOrDescriptor, path) assert.strictEqual(yield* fs.readFileString(path), "") @@ -181,8 +180,7 @@ export const testLayer = (layer: Layer.Layer, options: Test const result = yield* Effect.result(fs.copy(source, destination, { overwrite: false })) if (Result.isFailure(result)) { - assert(result.failure.reason instanceof PlatformError.SystemError) - assert.strictEqual(result.failure.reason._tag, "AlreadyExists") + assert(result.failure.reason._tag === "AlreadyExists") assert.strictEqual(result.failure.reason.method, "copy") assert.strictEqual(result.failure.reason.pathOrDescriptor, source) } diff --git a/packages/platform-deno/src/DenoFileSystem.ts b/packages/platform-deno/src/DenoFileSystem.ts index e8d61f91058..9e263a7c357 100644 --- a/packages/platform-deno/src/DenoFileSystem.ts +++ b/packages/platform-deno/src/DenoFileSystem.ts @@ -455,7 +455,7 @@ const writeFile: FileSystem.FileSystem["writeFile"] = (path, data, options) => { (file) => new FileImpl(file, flag.startsWith("a")).writeAll(data).pipe( Effect.mapError((error) => - error.reason instanceof PlatformError.SystemError + error.reason._tag !== "BadArgument" ? PlatformError.systemError({ ...error.reason, method: "writeFile", pathOrDescriptor: path }) : error ) From 6dfba015b3a6f48ca8476c5f78057769f3d3bdcb Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Thu, 6 Aug 2026 10:01:11 +0000 Subject: [PATCH 4/4] Restore EEXIST error mapping coverage --- packages/platform-deno/test/internal/error.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/platform-deno/test/internal/error.test.ts b/packages/platform-deno/test/internal/error.test.ts index 2605eec8dc6..991974ed999 100644 --- a/packages/platform-deno/test/internal/error.test.ts +++ b/packages/platform-deno/test/internal/error.test.ts @@ -10,6 +10,7 @@ describe("handleError", () => { [withCode(new Deno.errors.NotFound(), "ENOENT"), "NotFound"], [withCode(new Deno.errors.NotADirectory(), "ENOTDIR"), "BadResource"], [new Deno.errors.AlreadyExists(), "AlreadyExists"], + [withCode(new Deno.errors.AlreadyExists(), "EEXIST"), "AlreadyExists"], [withCode(new Deno.errors.IsADirectory(), "EISDIR"), "BadResource"], [withCode(new Deno.errors.PermissionDenied(), "EACCES"), "PermissionDenied"], [new Deno.errors.NotCapable(), "PermissionDenied"],