Skip to content

Commit 3add030

Browse files
fix: prevent fatal oo-editors write EPIPE crashes (#23)
* fix: handle stdio EPIPE without fatal crash * fix: keep broken pipe telemetry in breadcrumbs * fix: narrow stdio EPIPE handling * test: make broken pipe integration cross-platform * test: harden broken pipe probe detection * test: use direct broken pipe harness --------- Co-authored-by: Victor A. <52110451+cs50victor@users.noreply.github.com>
1 parent 01eba3e commit 3add030

5 files changed

Lines changed: 447 additions & 31 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
const { createStdIoState, handleStdIoStreamError } = require('../../server-lifecycle');
2+
3+
const mode = process.argv[2];
4+
const stdioState = createStdIoState();
5+
6+
function shutdown(reason, code) {
7+
process.stderr.write(`shutdown:${reason}:${code}\n`);
8+
process.exit(code);
9+
}
10+
11+
process.stdout.on('error', (error) => {
12+
handleStdIoStreamError('stdout', error, {
13+
stdioState,
14+
addBreadcrumb: () => {},
15+
captureException: () => {},
16+
shutdown
17+
});
18+
});
19+
20+
process.stderr.on('error', (error) => {
21+
handleStdIoStreamError('stderr', error, {
22+
stdioState,
23+
addBreadcrumb: () => {},
24+
captureException: () => {},
25+
shutdown
26+
});
27+
});
28+
29+
if (mode === 'stdout-broken') {
30+
console.log('first');
31+
const interval = setInterval(() => {
32+
console.log(`tick-${Date.now()}`);
33+
}, 10);
34+
35+
setTimeout(() => {
36+
clearInterval(interval);
37+
process.exit(99);
38+
}, 500);
39+
}

__tests__/server-lifecycle.test.js

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import { describe, test, expect } from 'bun:test';
2+
import { spawn } from 'child_process';
3+
import path from 'path';
4+
import {
5+
createStdIoState,
6+
handleProcessFailure,
7+
handleStdIoStreamError,
8+
isBrokenPipeError
9+
} from '../server-lifecycle.js';
10+
11+
function createBrokenPipeError() {
12+
const error = new Error('write EPIPE');
13+
error.code = 'EPIPE';
14+
error.errno = 'EPIPE';
15+
return error;
16+
}
17+
18+
function runBrokenPipeFixture(fixturePath, timeoutMs = 2000) {
19+
return new Promise((resolve, reject) => {
20+
const child = spawn('node', [fixturePath, 'stdout-broken'], {
21+
cwd: process.cwd(),
22+
stdio: ['ignore', 'pipe', 'pipe']
23+
});
24+
let stdout = '';
25+
let stderr = '';
26+
let closedStdout = false;
27+
let settled = false;
28+
29+
const timeout = setTimeout(() => {
30+
child.kill('SIGKILL');
31+
reject(new Error(`child timed out after ${timeoutMs}ms`));
32+
}, timeoutMs);
33+
34+
function closeStdoutAfterFirstLine() {
35+
if (closedStdout || !stdout.includes('\n')) {
36+
return;
37+
}
38+
39+
closedStdout = true;
40+
child.stdout.pause();
41+
child.stdout.destroy();
42+
}
43+
44+
child.stdout.on('data', (chunk) => {
45+
stdout += chunk.toString();
46+
closeStdoutAfterFirstLine();
47+
});
48+
child.stderr.on('data', (chunk) => {
49+
stderr += chunk.toString();
50+
});
51+
52+
child.on('error', (error) => {
53+
if (settled) {
54+
return;
55+
}
56+
settled = true;
57+
clearTimeout(timeout);
58+
reject(error);
59+
});
60+
61+
child.on('close', (code, signal) => {
62+
if (settled) {
63+
return;
64+
}
65+
settled = true;
66+
clearTimeout(timeout);
67+
resolve({ code, signal, stdout, stderr, closedStdout });
68+
});
69+
});
70+
}
71+
72+
describe('isBrokenPipeError', () => {
73+
test('detects common broken-pipe shapes', () => {
74+
expect(isBrokenPipeError(createBrokenPipeError())).toBe(true);
75+
expect(isBrokenPipeError({ errno: 'EPIPE' })).toBe(true);
76+
expect(isBrokenPipeError(new Error('stream failed with EPIPE'))).toBe(true);
77+
});
78+
79+
test('ignores unrelated errors', () => {
80+
expect(isBrokenPipeError(new Error('permission denied'))).toBe(false);
81+
expect(isBrokenPipeError({ code: 'ENOENT' })).toBe(false);
82+
});
83+
});
84+
85+
describe('createStdIoState', () => {
86+
test('suppresses repeated writes after an stdout broken pipe and emits telemetry once', () => {
87+
const telemetry = [];
88+
const stdioState = createStdIoState({
89+
onBrokenPipeTelemetry: (details) => telemetry.push(details)
90+
});
91+
const fakeConsole = {
92+
log() {
93+
throw createBrokenPipeError();
94+
}
95+
};
96+
97+
expect(stdioState.writeLog('log', 'stdout', 'first', [], fakeConsole)).toBe(false);
98+
expect(stdioState.writeLog('log', 'stdout', 'second', [], fakeConsole)).toBe(false);
99+
expect(telemetry).toHaveLength(1);
100+
expect(stdioState.state).toMatchObject({
101+
stdoutBrokenPipe: true,
102+
stderrBrokenPipe: false,
103+
brokenPipeTelemetrySent: true
104+
});
105+
});
106+
});
107+
108+
describe('handleStdIoStreamError', () => {
109+
test('gracefully shuts down on stdout EPIPE', () => {
110+
const telemetry = [];
111+
const shutdownCalls = [];
112+
const stdioState = createStdIoState({
113+
onBrokenPipeTelemetry: (details) => telemetry.push(details)
114+
});
115+
116+
handleStdIoStreamError('stdout', createBrokenPipeError(), {
117+
stdioState,
118+
addBreadcrumb: () => {},
119+
captureException: () => {},
120+
shutdown: (reason, code) => shutdownCalls.push({ reason, code })
121+
});
122+
123+
expect(shutdownCalls).toEqual([{ reason: 'stdout-epipe', code: 0 }]);
124+
expect(telemetry).toHaveLength(1);
125+
});
126+
127+
test('keeps non-broken-pipe stdio errors fatal', () => {
128+
const breadcrumbs = [];
129+
const exceptions = [];
130+
const shutdownCalls = [];
131+
const error = new Error('stream failure');
132+
133+
handleStdIoStreamError('stderr', error, {
134+
stdioState: createStdIoState(),
135+
addBreadcrumb: (...args) => breadcrumbs.push(args),
136+
captureException: (...args) => exceptions.push(args),
137+
shutdown: (reason, code) => shutdownCalls.push({ reason, code })
138+
});
139+
140+
expect(breadcrumbs).toHaveLength(1);
141+
expect(exceptions).toHaveLength(1);
142+
expect(shutdownCalls).toEqual([{ reason: 'stderr-error', code: 1 }]);
143+
});
144+
});
145+
146+
describe('handleProcessFailure', () => {
147+
test('keeps uncaught EPIPE fatal outside stdio handlers', () => {
148+
const shutdownCalls = [];
149+
const exceptions = [];
150+
const logCalls = [];
151+
152+
handleProcessFailure('uncaughtException', createBrokenPipeError(), {
153+
addBreadcrumb: () => {},
154+
captureException: (...args) => exceptions.push(args),
155+
logError: (...args) => logCalls.push(args),
156+
shutdown: (reason, code) => shutdownCalls.push({ reason, code })
157+
});
158+
159+
expect(exceptions).toHaveLength(1);
160+
expect(logCalls).toHaveLength(1);
161+
expect(shutdownCalls).toEqual([{ reason: 'uncaughtException', code: 1 }]);
162+
});
163+
164+
test('keeps unhandledRejection EPIPE fatal outside stdio handlers', () => {
165+
const shutdownCalls = [];
166+
const exceptions = [];
167+
const logCalls = [];
168+
169+
handleProcessFailure('unhandledRejection', createBrokenPipeError(), {
170+
addBreadcrumb: () => {},
171+
captureException: (...args) => exceptions.push(args),
172+
logError: (...args) => logCalls.push(args),
173+
shutdown: (reason, code) => shutdownCalls.push({ reason, code })
174+
});
175+
176+
expect(exceptions).toHaveLength(1);
177+
expect(logCalls).toHaveLength(1);
178+
expect(shutdownCalls).toEqual([{ reason: 'unhandledRejection', code: 1 }]);
179+
});
180+
});
181+
182+
describe('broken pipe integration', () => {
183+
test('exits cleanly after stdout closes', async () => {
184+
const fixturePath = path.join(process.cwd(), '__tests__', 'fixtures', 'stdio-lifecycle-child.js');
185+
const result = await runBrokenPipeFixture(fixturePath);
186+
expect(result.closedStdout).toBe(true);
187+
expect(result.stdout.startsWith('first\n')).toBe(true);
188+
expect(result.code).toBe(0);
189+
expect(result.stderr).toContain('shutdown:stdout-epipe:0');
190+
});
191+
});

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"server": "cross-env FONT_DATA_DIR=assets/onlyoffice-fontdata node server.js",
88
"test": "bun run test:unit && bun run test:e2e",
99
"test:all": "bun run test:unit && bun run test:e2e",
10-
"test:unit": "bun test __tests__/server-utils.test.js __tests__/desktop-stub-utils.test.js __tests__/generate-office-fonts-path.test.js __tests__/offline-loader-config.test.js && node test-url-scheme.js",
10+
"test:unit": "bun test __tests__/server-utils.test.js __tests__/server-lifecycle.test.js __tests__/desktop-stub-utils.test.js __tests__/generate-office-fonts-path.test.js __tests__/offline-loader-config.test.js && node test-url-scheme.js",
1111
"test:e2e": "bun run test:console-batch && bun run test:logo && bun run test:save",
1212
"test:console-batch": "node test-console-batch.js",
1313
"test:logo": "node __tests__/logo-header.test.js",

0 commit comments

Comments
 (0)