Repository navigation
fix(desktop): Windows updates no longer quit into the installer without disk space #27526
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
2bce178
4933c12
e23bd9a
909389d
b3500d2
0ab5dbe
63ec6dc
c75f235
951c3bf
d43fa7f
279691a
03e5778
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import * as NodeServices from "@effect/platform-node/NodeServices"; | ||
| import { assert, describe, it } from "@effect/vitest"; | ||
| import * as Effect from "effect/Effect"; | ||
| import * as Exit from "effect/Exit"; | ||
| import * as FileSystem from "effect/FileSystem"; | ||
| import * as Layer from "effect/Layer"; | ||
| import * as Path from "effect/Path"; | ||
|
|
||
| import * as DesktopDiskSpace from "./DesktopDiskSpace.ts"; | ||
|
|
||
| describe("DesktopDiskSpace", () => { | ||
| it.effect("sums nested files without following symlinks", () => | ||
| Effect.gen(function* () { | ||
| const fileSystem = yield* FileSystem.FileSystem; | ||
| const path = yield* Path.Path; | ||
| const diskSpace = yield* DesktopDiskSpace.DesktopDiskSpace; | ||
| const root = yield* fileSystem.makeTempDirectoryScoped(); | ||
| const outside = yield* fileSystem.makeTempDirectoryScoped(); | ||
| yield* fileSystem.makeDirectory(path.join(root, "resources", "app"), { recursive: true }); | ||
| yield* fileSystem.writeFileString(path.join(root, "app.exe"), "x".repeat(100)); | ||
| yield* fileSystem.writeFileString( | ||
| path.join(root, "resources", "app", "a.js"), | ||
| "x".repeat(20), | ||
| ); | ||
| yield* fileSystem.writeFileString(path.join(outside, "big.bin"), "x".repeat(5000)); | ||
| yield* fileSystem.symlink(path.join(outside, "big.bin"), path.join(root, "big-link")); | ||
| yield* fileSystem.symlink(outside, path.join(root, "outside-link")); | ||
|
|
||
| assert.equal(yield* diskSpace.directoryBytes(root), 120); | ||
| assert.isTrue( | ||
| Exit.isFailure(yield* Effect.exit(diskSpace.directoryBytes(path.join(root, "missing")))), | ||
| ); | ||
| }).pipe(Effect.scoped, Effect.provide(Layer.merge(DesktopDiskSpace.layer, NodeServices.layer))), | ||
| ); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| // @effect-diagnostics nodeBuiltinImport:off -- FileSystem has no statfs, and its stat follows symlinks. | ||
| import * as NodeFS from "node:fs"; | ||
| import * as NodeOS from "node:os"; | ||
| import * as NodePath from "node:path"; | ||
|
|
||
| import * as Context from "effect/Context"; | ||
| import * as Effect from "effect/Effect"; | ||
| import * as Layer from "effect/Layer"; | ||
| import * as Schema from "effect/Schema"; | ||
|
|
||
| const MAX_DIRECTORY_ENTRIES = 100_000; | ||
|
|
||
| // Electron's fs reports .asar archives as empty directories; original-fs sees the real files. | ||
| const NodeFSP = ((process.getBuiltinModule("original-fs") as typeof NodeFS | undefined) ?? NodeFS) | ||
| .promises; | ||
|
|
||
| export class DesktopDiskSpaceError extends Schema.TaggedError<DesktopDiskSpaceError>()( | ||
| "DesktopDiskSpaceError", | ||
| { | ||
| operation: Schema.Literals(["freeBytes", "directoryBytes"]), | ||
| cause: Schema.Defect(), | ||
| }, | ||
| ) { | ||
| override get message(): string { | ||
| return `Could not measure disk space (${this.operation}).`; | ||
| } | ||
| } | ||
|
|
||
| export class DesktopDiskSpace extends Context.Service< | ||
| DesktopDiskSpace, | ||
| { | ||
| /** The temp directory a spawned installer inherits. */ | ||
| readonly tempDirectory: string; | ||
| /** Bytes the current user can still write on the volume holding `path`. */ | ||
| readonly freeBytes: (path: string) => Effect.Effect<number, DesktopDiskSpaceError>; | ||
| /** Total size of the regular files below `path`, without following symlinks. */ | ||
| readonly directoryBytes: (path: string) => Effect.Effect<number, DesktopDiskSpaceError>; | ||
| } | ||
| >()("@t3tools/desktop/updates/DesktopDiskSpace") {} | ||
|
|
||
| async function sumDirectoryBytes(root: string): Promise<number> { | ||
| let total = 0; | ||
| let entries = 0; | ||
| const pending = [root]; | ||
| for (let directory = pending.pop(); directory !== undefined; directory = pending.pop()) { | ||
| // A partial total would understate the space the installer needs, so any failure fails the walk. | ||
| const children = await NodeFSP.readdir(directory, { withFileTypes: true }); | ||
| entries += children.length; | ||
| if (entries > MAX_DIRECTORY_ENTRIES) { | ||
| throw new Error(`More than ${MAX_DIRECTORY_ENTRIES} entries below the directory.`); | ||
| } | ||
| for (const child of children) { | ||
| const childPath = NodePath.join(directory, child.name); | ||
| if (child.isDirectory()) pending.push(childPath); | ||
| else if (child.isFile()) total += (await NodeFSP.lstat(childPath)).size; | ||
| } | ||
| } | ||
| return total; | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Posted via Macroscope — Effect Service Conventions |
||
| export const layer = Layer.sync(DesktopDiskSpace, () => ({ | ||
| tempDirectory: NodeOS.tmpdir(), | ||
| freeBytes: Effect.fn("DesktopDiskSpace.freeBytes")(function* (path: string) { | ||
| const stats = yield* Effect.tryPromise({ | ||
| try: () => NodeFSP.statfs(path), | ||
| catch: (cause) => new DesktopDiskSpaceError({ operation: "freeBytes", cause }), | ||
| }); | ||
| return stats.bavail * stats.bsize; | ||
| }), | ||
| directoryBytes: Effect.fn("DesktopDiskSpace.directoryBytes")(function* (path: string) { | ||
| return yield* Effect.tryPromise({ | ||
| try: () => sumDirectoryBytes(path), | ||
| catch: (cause) => new DesktopDiskSpaceError({ operation: "directoryBytes", cause }), | ||
| }); | ||
| }), | ||
| })); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -727,6 +727,95 @@ describe("DesktopUpdates", () => { | |
| ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); | ||
| }); | ||
|
|
||
| // regression: #27486 a silent NSIS update on a full volume left a half-written install that no longer launched | ||
| it.effect("keeps a Windows update retryable when the temp volume lacks space", () => { | ||
| const megabyte = 1024 * 1024; | ||
| const diskSpace = { | ||
| tempFreeBytes: 200 * megabyte, | ||
| installerBytes: 150 * megabyte, | ||
| installDirectoryBytes: 400 * megabyte, | ||
| }; | ||
| const harness = makeHarness({ platform: "win32", diskSpace }); | ||
|
|
||
| return Effect.scoped( | ||
| Effect.gen(function* () { | ||
| const desktopState = yield* DesktopState.DesktopState; | ||
| const pool = yield* DesktopBackendPool.DesktopBackendPool; | ||
| const updates = yield* DesktopUpdates.DesktopUpdates; | ||
| yield* updates.configure; | ||
| harness.emit("update-downloaded", { | ||
| version: "1.2.4", | ||
| downloadedFile: "C:\\Users\\me\\AppData\\Local\\t3code-updater\\pending\\installer.exe", | ||
| }); | ||
| yield* flushCallbacks; | ||
|
|
||
| const result = yield* updates.install; | ||
| assert.equal(harness.quitAndInstalls(), 0, "the installer ran without enough disk space"); | ||
| assert.isFalse(yield* Ref.get(desktopState.quitting)); | ||
| assert.isTrue((yield* (yield* pool.primary).snapshot).desiredRunning); | ||
| assert.isFalse(result.completed); | ||
| assert.equal(result.state.status, "downloaded"); | ||
| assert.equal(result.state.downloadedVersion, "1.2.4"); | ||
| assert.equal(result.state.errorContext, "install"); | ||
| assert.isTrue(result.state.canRetry); | ||
| assert.match(result.state.message ?? "", /space/i); | ||
|
|
||
| diskSpace.tempFreeBytes = 2000 * megabyte; | ||
| const retry = yield* updates.install; | ||
| assert.equal(harness.quitAndInstalls(), 1); | ||
| assert.isNull(retry.state.message, "a successful retry still reports the earlier refusal"); | ||
| }), | ||
| ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); | ||
| }); | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This repeats the temp-volume refusal and successful retry in the preceding test. Could you use Posted via Macroscope — Test Signal |
||
| it.effect("keeps a Windows update retryable when a separate install volume is full", () => { | ||
| const megabyte = 1024 * 1024; | ||
| const harness = makeHarness({ | ||
| platform: "win32", | ||
| diskSpace: { | ||
| tempFreeBytes: 2000 * megabyte, | ||
| installFreeBytes: 200 * megabyte, | ||
| installerBytes: 150 * megabyte, | ||
| installDirectoryBytes: 400 * megabyte, | ||
| }, | ||
| }); | ||
|
|
||
| return Effect.scoped( | ||
| Effect.gen(function* () { | ||
| const updates = yield* DesktopUpdates.DesktopUpdates; | ||
| yield* updates.configure; | ||
| harness.emit("update-downloaded", { | ||
| version: "1.2.4", | ||
| downloadedFile: "C:\\Users\\me\\AppData\\Local\\t3code-updater\\pending\\installer.exe", | ||
| }); | ||
| yield* flushCallbacks; | ||
|
|
||
| const result = yield* updates.install; | ||
| assert.equal(harness.quitAndInstalls(), 0); | ||
| assert.match(result.state.message ?? "", /space/i); | ||
| }), | ||
| ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); | ||
| }); | ||
|
|
||
| it.effect("installs a Windows update when disk space cannot be measured", () => { | ||
| const harness = makeHarness({ platform: "win32" }); | ||
|
|
||
| return Effect.scoped( | ||
| Effect.gen(function* () { | ||
| const updates = yield* DesktopUpdates.DesktopUpdates; | ||
| yield* updates.configure; | ||
| harness.emit("update-downloaded", { | ||
| version: "1.2.4", | ||
| downloadedFile: "C:\\Users\\me\\AppData\\Local\\t3code-updater\\pending\\installer.exe", | ||
| }); | ||
| yield* flushCallbacks; | ||
|
|
||
| yield* updates.install; | ||
| assert.equal(harness.quitAndInstalls(), 1, "an unmeasured volume blocked the update"); | ||
| }), | ||
| ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); | ||
| }); | ||
|
|
||
| it.effect("restores only desired-running instances after an installer error event", () => { | ||
| const starts: string[] = []; | ||
| const stops: string[] = []; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extends the existing retained-download failure case above. Could you fold this assertion into that test and rename it to cover both failure contexts?
Posted via Macroscope — Test Signal