Skip to content

Commit 3f82e62

Browse files
NitayRabiclaude
andcommitted
refactor(gateway): inject platform into isMuslLibc, broaden exit-as-failure
Address review feedback on #36: - isMuslLibc(platform = process.platform): accept the platform as an argument so tests don't need to mutate process.platform (which is non-configurable in some Node versions and would have thrown there). All 6 tests now pass the platform explicitly. - Spawn 'exit' handler: previously only recorded spawnFailure when code !== 0 && code !== null, so exits via signal (code===null, e.g. OOM) or a clean exit-0 before readiness still fell through to the generic 30s timeout. Now records spawnFailure for any exit-before- ready, with the descriptor adapting to "code N" vs "signal X". Also guarded against overwriting a spawn-error reason that already populated spawnFailure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 84739bc commit 3f82e62

2 files changed

Lines changed: 21 additions & 23 deletions

File tree

src/gateway-manager.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,10 @@ export class IBGatewayManager {
156156
// Detect whether the current Linux system uses musl libc (Alpine, etc.) rather than glibc.
157157
// The bundled glibc JRE cannot exec on musl — its ELF interpreter /lib64/ld-linux-x86-64.so.2
158158
// does not exist there, producing an opaque ENOENT at spawn time.
159-
static isMuslLibc(): boolean {
160-
if (process.platform !== 'linux') {
159+
// The platform argument is injected so tests can exercise the matrix without mutating
160+
// process.platform (which is non-configurable on some Node versions).
161+
static isMuslLibc(platform: NodeJS.Platform = process.platform): boolean {
162+
if (platform !== 'linux') {
161163
return false;
162164
}
163165
// process.report.getReport() exposes glibcVersionRuntime when glibc is present.
@@ -384,10 +386,16 @@ export class IBGatewayManager {
384386

385387
this.gatewayProcess.on('exit', (code, signal) => {
386388
this.log(`🛑 Gateway process exited with code ${code}, signal ${signal}`);
387-
if (!this.isReady && code !== 0 && code !== null) {
389+
// Any exit before the gateway became ready is a failure: a clean exit-0 means the JVM
390+
// terminated without ever opening the HTTP port (e.g. config error), and code===null
391+
// means the process was killed by a signal (e.g. OOM). Recording spawnFailure for all
392+
// of these makes waitForGateway() bail out fast instead of polling for 30s.
393+
if (!this.isReady && !this.spawnFailure) {
394+
const exitDescriptor =
395+
code !== null ? `code ${code}` : `signal ${signal ?? 'unknown'}`;
388396
this.spawnFailure = {
389-
reason: `IB Gateway process exited with code ${code} before becoming ready`,
390-
details: stderrTail.trim() || `(no stderr captured; signal=${signal ?? 'none'})`,
397+
reason: `IB Gateway process exited (${exitDescriptor}) before becoming ready`,
398+
details: stderrTail.trim() || `(no stderr captured)`,
391399
};
392400
}
393401
this.gatewayProcess = null;

test/gateway-manager.test.ts

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,66 +12,56 @@ vi.mock('fs', async () => {
1212
});
1313

1414
describe('IBGatewayManager.isMuslLibc', () => {
15-
const originalPlatform = process.platform;
1615
const originalReport = (process as unknown as { report?: unknown }).report;
1716

1817
afterEach(() => {
19-
Object.defineProperty(process, 'platform', { value: originalPlatform });
2018
Object.defineProperty(process, 'report', { configurable: true, value: originalReport });
2119
vi.mocked(fs.existsSync).mockReset();
22-
vi.mocked(fs.existsSync).mockImplementation((p: fs.PathLike) => {
23-
// Default: nothing exists unless a test opts in.
24-
return false;
25-
});
20+
vi.mocked(fs.existsSync).mockImplementation(() => false);
2621
});
2722

2823
it('returns false on non-Linux platforms without consulting libc', () => {
29-
Object.defineProperty(process, 'platform', { value: 'darwin' });
3024
vi.mocked(fs.existsSync).mockReturnValue(true); // would lie if consulted
31-
expect(IBGatewayManager.isMuslLibc()).toBe(false);
25+
expect(IBGatewayManager.isMuslLibc('darwin')).toBe(false);
26+
expect(IBGatewayManager.isMuslLibc('win32')).toBe(false);
3227
});
3328

3429
it('returns false on Linux when process.report exposes a glibc runtime version', () => {
35-
Object.defineProperty(process, 'platform', { value: 'linux' });
3630
Object.defineProperty(process, 'report', { configurable: true, value: {
3731
getReport: () => ({ header: { glibcVersionRuntime: '2.36' } }),
3832
} });
39-
expect(IBGatewayManager.isMuslLibc()).toBe(false);
33+
expect(IBGatewayManager.isMuslLibc('linux')).toBe(false);
4034
});
4135

4236
it('returns true on Linux when glibcVersionRuntime is missing and the musl loader is on disk', () => {
43-
Object.defineProperty(process, 'platform', { value: 'linux' });
4437
Object.defineProperty(process, 'report', { configurable: true, value: {
4538
getReport: () => ({ header: {} }),
4639
} });
4740
vi.mocked(fs.existsSync).mockImplementation(
4841
(p: fs.PathLike) => p === '/lib/ld-musl-x86_64.so.1',
4942
);
50-
expect(IBGatewayManager.isMuslLibc()).toBe(true);
43+
expect(IBGatewayManager.isMuslLibc('linux')).toBe(true);
5144
});
5245

5346
it('returns true when only the aarch64 musl loader is present', () => {
54-
Object.defineProperty(process, 'platform', { value: 'linux' });
5547
Object.defineProperty(process, 'report', { configurable: true, value: {
5648
getReport: () => ({ header: {} }),
5749
} });
5850
vi.mocked(fs.existsSync).mockImplementation(
5951
(p: fs.PathLike) => p === '/lib/ld-musl-aarch64.so.1',
6052
);
61-
expect(IBGatewayManager.isMuslLibc()).toBe(true);
53+
expect(IBGatewayManager.isMuslLibc('linux')).toBe(true);
6254
});
6355

6456
it('returns false when neither glibc nor a musl loader is detectable', () => {
65-
Object.defineProperty(process, 'platform', { value: 'linux' });
6657
Object.defineProperty(process, 'report', { configurable: true, value: {
6758
getReport: () => ({ header: {} }),
6859
} });
6960
vi.mocked(fs.existsSync).mockReturnValue(false);
70-
expect(IBGatewayManager.isMuslLibc()).toBe(false);
61+
expect(IBGatewayManager.isMuslLibc('linux')).toBe(false);
7162
});
7263

7364
it('falls back to the filesystem check if process.report.getReport throws', () => {
74-
Object.defineProperty(process, 'platform', { value: 'linux' });
7565
Object.defineProperty(process, 'report', { configurable: true, value: {
7666
getReport: () => {
7767
throw new Error('not available');
@@ -80,6 +70,6 @@ describe('IBGatewayManager.isMuslLibc', () => {
8070
vi.mocked(fs.existsSync).mockImplementation(
8171
(p: fs.PathLike) => p === '/lib/ld-musl-x86_64.so.1',
8272
);
83-
expect(IBGatewayManager.isMuslLibc()).toBe(true);
73+
expect(IBGatewayManager.isMuslLibc('linux')).toBe(true);
8474
});
8575
});

0 commit comments

Comments
 (0)