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
1 change: 1 addition & 0 deletions apps/desktop/src/app/DesktopConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const DesktopConfig = Config.all({
),
appImagePath: trimmedString("APPIMAGE"),
disableAutoUpdate: optionalBoolean("T3CODE_DISABLE_AUTO_UPDATE"),
supervisedRelaunch: optionalBoolean("T3CODE_SUPERVISED_RELAUNCH"),
mockUpdates: optionalBoolean("T3CODE_DESKTOP_MOCK_UPDATES"),
mockUpdateServerPort: Config.Port("T3CODE_DESKTOP_MOCK_UPDATE_SERVER_PORT").pipe(
Config.withDefault(3000),
Expand Down
28 changes: 28 additions & 0 deletions apps/desktop/src/backend/DesktopBackendManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,34 @@ describe("DesktopBackendManager", () => {
}),
);

it.effect("keeps the underlying cause when an exit status cannot be read", () =>
Effect.gen(function* () {
const exitCause = PlatformError.systemError({
_tag: "Unknown",
module: "ChildProcess",
method: "exitCode",
cause: new Error("waitpid failed: ECHILD"),
});
const layerSpawner = Layer.succeed(
ChildProcessSpawner.ChildProcessSpawner,
ChildProcessSpawner.make(() =>
Effect.succeed(makeProcess({ exitCode: Effect.fail(exitCause) })),
),
);

const error = yield* DesktopBackendManager.runBackendProcess({
...baseConfig,
desktopTelemetryStream: Stream.empty,
}).pipe(
Effect.scoped,
Effect.provide(Layer.merge(layerSpawner, layerHealthyHttpClient)),
Effect.flip,
);

assert.include(error.message, "waitpid failed: ECHILD");
}),
);

it.effect("reports child exit before waiting for trailing output to drain", () =>
Effect.scoped(
Effect.gen(function* () {
Expand Down
13 changes: 12 additions & 1 deletion apps/desktop/src/backend/DesktopBackendManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,17 @@ export type BackendProcessOutputError =
| BackendProcessOutputReadError
| BackendProcessOutputHandlingError;

// The spawner wraps the useful detail (errno, signal text) a level or two down.
function describeErrorChain(error: unknown): string {
const messages: string[] = [];
let current = error;
while (current instanceof Error && messages.length < 3) {
if (current.message !== "") messages.push(current.message);
current = current.cause;
}
return messages.length > 0 ? messages.join(": ") : String(error);
}

export class BackendProcessExitStatusError extends Schema.TaggedError<BackendProcessExitStatusError>()(
"BackendProcessExitStatusError",
{
Expand All @@ -174,7 +185,7 @@ export class BackendProcessExitStatusError extends Schema.TaggedError<BackendPro
},
) {
override get message(): string {
return `Failed to read the exit status of desktop backend process ${this.pid}.`;
return `Failed to read the exit status of desktop backend process ${this.pid}: ${describeErrorChain(this.cause)}`;
}
}

Expand Down
10 changes: 10 additions & 0 deletions apps/desktop/src/telemetry/DesktopRendererHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ interface TrackedRenderer {
creationTimeMs: number | null;
memory: RendererMemory | null;
gone: boolean;
deadSampleRecorded: boolean;
readonly removeListeners: () => void;
}

Expand Down Expand Up @@ -218,6 +219,7 @@ const make = Effect.fn("DesktopRendererHistory.make")(function* () {
creationTimeMs: null,
memory: null,
gone: false,
deadSampleRecorded: false,
removeListeners: () => {
webContents.removeListener("dom-ready", onDomReady);
webContents.removeListener("devtools-opened", onDevToolsOpened);
Expand Down Expand Up @@ -260,6 +262,14 @@ const make = Effect.fn("DesktopRendererHistory.make")(function* () {
peakWorkingSetBytes: nonnegative(metric.memory.peakWorkingSetSize * 1024) ?? 0,
};
}
// A dead renderer would otherwise log an identical row every 30 s and
// rotate the record of how it died out of the bounded history.
if (pidSource !== "current") {
if (renderer.deadSampleRecorded) continue;
renderer.deadSampleRecorded = true;
} else {
renderer.deadSampleRecorded = false;
}
// A process can host multiple contents; these rows must not be summed.
record(renderer, "sample");
}
Expand Down
17 changes: 17 additions & 0 deletions apps/desktop/src/telemetry/DesktopTelemetryPublisher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,28 @@ describe("DesktopRendererHistory", () => {
assert.equal(crash.rendererPidSource, "last-known");
assert.equal(crash.reason, "oom");
assert.equal(crash.exitCode, -7);
const deadSampled = yield* Deferred.make<Record>();
milestone = {
matches: (row) => row.event === "sample" && row.webContentsId === 2,
written: deadSampled,
};
yield* TestClock.adjust(Duration.seconds(30));
yield* history.recordMetrics(metrics);
assert.equal((yield* Deferred.await(deadSampled)).rendererPidSource, "last-known");
const afterDeadSample = persisted.length;
yield* TestClock.adjust(Duration.seconds(30));
yield* history.recordMetrics(metrics);
const restarted = yield* Deferred.make<Record>();
milestone = { matches: (row) => row.event === "dom-ready", written: restarted };
previewPid = 7_003;
preview.emit("dom-ready");
const restart = yield* Deferred.await(restarted);
// Records are written in order, so the dom-ready row lands after any repeat sample.
assert.isFalse(
persisted
.slice(afterDeadSample)
.some((row) => row.event === "sample" && row.webContentsId === 2),
);
assert.equal(restart.rendererPid, 7_003);
assert.equal(restart.rendererPidSource, "current");
assert.isNull(restart.rendererCreationTimeMs);
Expand Down
21 changes: 21 additions & 0 deletions apps/desktop/src/updates/DesktopUpdates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,27 @@ describe("DesktopUpdates", () => {
).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
});

it.effect.each([
{ env: {}, isForceRunAfter: true },
{ env: { T3CODE_SUPERVISED_RELAUNCH: "true" }, isForceRunAfter: false },
])(
"leaves the relaunch to a supervisor only when told one owns it",
({ env, isForceRunAfter }) => {
const harness = makeHarness({ env });
return Effect.scoped(
Effect.gen(function* () {
const updates = yield* DesktopUpdates.DesktopUpdates;
yield* updates.configure;
harness.emit("update-downloaded", { version: "1.2.4" });
yield* flushCallbacks;

assert.isTrue((yield* updates.install).accepted);
assert.deepEqual(harness.quitAndInstallOptions, [{ isForceRunAfter }]);
}),
).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
},
);

it.effect("records the version an install hands to the installer", () => {
let recordsAtStop: ReadonlyArray<string> = [];
const harness = makeHarness({
Expand Down
5 changes: 4 additions & 1 deletion apps/desktop/src/updates/DesktopUpdates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,10 @@ export const make = Effect.gen(function* () {
);
yield* electronUpdater.quitAndInstall({
isSilent: true,
isForceRunAfter: true,
// A supervisor that restarts the app on exit (a systemd unit with
// Restart=) must own the relaunch: a copy the updater spawns runs
// outside the unit, without its limits, and the unit loses track of it.
isForceRunAfter: !config.supervisedRelaunch,
});
return { accepted: true, completed: false, failed: false };
}).pipe(
Expand Down
5 changes: 4 additions & 1 deletion apps/desktop/src/updates/updatesTestHarness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface UpdatesHarnessOptions {
export function makeHarness(options: UpdatesHarnessOptions = {}) {
let checkCount = 0;
let quitAndInstallCount = 0;
const quitAndInstallOptions: Array<{ readonly isForceRunAfter: boolean }> = [];
let downloadCount = 0;
let allowDowngrade = false;
let fullChangelog = false;
Expand Down Expand Up @@ -90,9 +91,10 @@ export function makeHarness(options: UpdatesHarnessOptions = {}) {
downloadUpdate: Effect.sync(() => {
downloadCount += 1;
}).pipe(Effect.andThen(options.downloadUpdate ?? Effect.void)),
quitAndInstall: () =>
quitAndInstall: (installOptions) =>
Effect.sync(() => {
quitAndInstallCount += 1;
quitAndInstallOptions.push({ isForceRunAfter: installOptions.isForceRunAfter });
installSteps.push("quitAndInstall");
}).pipe(Effect.andThen(options.quitAndInstall ?? Effect.void)),
on: (eventName, listener) =>
Expand Down Expand Up @@ -273,6 +275,7 @@ export function makeHarness(options: UpdatesHarnessOptions = {}) {
layer,
checkCount: () => checkCount,
quitAndInstalls: () => quitAndInstallCount,
quitAndInstallOptions,
installSteps,
updateRestartMarkers,
installRecords,
Expand Down
Loading