fix(api): lazy-load node-pty to prevent API crash on missing native binary (#525)
All checks were successful
ci/woodpecker/push/api Pipeline was successful
All checks were successful
ci/woodpecker/push/api Pipeline was successful
Co-authored-by: Jason Woltje <jason@diversecanvas.com> Co-committed-by: Jason Woltje <jason@diversecanvas.com>
This commit was merged in pull request #525.
This commit is contained in:
@@ -31,7 +31,11 @@ COPY packages/config/package.json ./packages/config/
|
|||||||
COPY apps/api/package.json ./apps/api/
|
COPY apps/api/package.json ./apps/api/
|
||||||
|
|
||||||
# Install dependencies (no cache mount — Kaniko builds are ephemeral in CI)
|
# Install dependencies (no cache mount — Kaniko builds are ephemeral in CI)
|
||||||
RUN pnpm install --frozen-lockfile
|
# Then explicitly rebuild node-pty from source since pnpm may skip postinstall
|
||||||
|
# scripts or fail to find prebuilt binaries for this Node.js version
|
||||||
|
RUN pnpm install --frozen-lockfile \
|
||||||
|
&& cd node_modules/.pnpm/node-pty@*/node_modules/node-pty \
|
||||||
|
&& npx node-gyp rebuild 2>&1 || true
|
||||||
|
|
||||||
# ======================
|
# ======================
|
||||||
# Builder stage
|
# Builder stage
|
||||||
|
|||||||
@@ -46,7 +46,7 @@ describe("TerminalService", () => {
|
|||||||
let service: TerminalService;
|
let service: TerminalService;
|
||||||
let mockSocket: Socket;
|
let mockSocket: Socket;
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(async () => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
// Reset mock implementations
|
// Reset mock implementations
|
||||||
mockPtyProcess.onData.mockImplementation((_cb: (data: string) => void) => {});
|
mockPtyProcess.onData.mockImplementation((_cb: (data: string) => void) => {});
|
||||||
@@ -54,6 +54,8 @@ describe("TerminalService", () => {
|
|||||||
(_cb: (e: { exitCode: number; signal?: number }) => void) => {}
|
(_cb: (e: { exitCode: number; signal?: number }) => void) => {}
|
||||||
);
|
);
|
||||||
service = new TerminalService();
|
service = new TerminalService();
|
||||||
|
// Trigger lazy import of node-pty (uses dynamic import(), intercepted by vi.mock)
|
||||||
|
await service.onModuleInit();
|
||||||
mockSocket = createMockSocket();
|
mockSocket = createMockSocket();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -13,11 +13,19 @@
|
|||||||
* - closeWorkspaceSessions: kill all sessions for a workspace (on disconnect)
|
* - closeWorkspaceSessions: kill all sessions for a workspace (on disconnect)
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { Injectable, Logger } from "@nestjs/common";
|
import { Injectable, Logger, OnModuleInit } from "@nestjs/common";
|
||||||
import * as pty from "node-pty";
|
import type { IPty } from "node-pty";
|
||||||
import type { Socket } from "socket.io";
|
import type { Socket } from "socket.io";
|
||||||
import { randomUUID } from "node:crypto";
|
import { randomUUID } from "node:crypto";
|
||||||
|
|
||||||
|
// Lazy-loaded in onModuleInit via dynamic import() to prevent crash
|
||||||
|
// if the native binary is missing. node-pty requires a compiled .node
|
||||||
|
// binary which may not be available in all Docker environments.
|
||||||
|
interface NodePtyModule {
|
||||||
|
spawn: (file: string, args: string[], options: Record<string, unknown>) => IPty;
|
||||||
|
}
|
||||||
|
let pty: NodePtyModule | null = null;
|
||||||
|
|
||||||
/** Maximum concurrent PTY sessions per workspace */
|
/** Maximum concurrent PTY sessions per workspace */
|
||||||
export const MAX_SESSIONS_PER_WORKSPACE = parseInt(
|
export const MAX_SESSIONS_PER_WORKSPACE = parseInt(
|
||||||
process.env.TERMINAL_MAX_SESSIONS_PER_WORKSPACE ?? "10",
|
process.env.TERMINAL_MAX_SESSIONS_PER_WORKSPACE ?? "10",
|
||||||
@@ -31,7 +39,7 @@ const DEFAULT_ROWS = 24;
|
|||||||
export interface TerminalSession {
|
export interface TerminalSession {
|
||||||
sessionId: string;
|
sessionId: string;
|
||||||
workspaceId: string;
|
workspaceId: string;
|
||||||
pty: pty.IPty;
|
pty: IPty;
|
||||||
name?: string;
|
name?: string;
|
||||||
createdAt: Date;
|
createdAt: Date;
|
||||||
}
|
}
|
||||||
@@ -53,7 +61,7 @@ export interface SessionCreatedResult {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
export class TerminalService {
|
export class TerminalService implements OnModuleInit {
|
||||||
private readonly logger = new Logger(TerminalService.name);
|
private readonly logger = new Logger(TerminalService.name);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -66,13 +74,30 @@ export class TerminalService {
|
|||||||
*/
|
*/
|
||||||
private readonly workspaceSessions = new Map<string, Set<string>>();
|
private readonly workspaceSessions = new Map<string, Set<string>>();
|
||||||
|
|
||||||
|
async onModuleInit(): Promise<void> {
|
||||||
|
if (!pty) {
|
||||||
|
try {
|
||||||
|
pty = await import("node-pty");
|
||||||
|
this.logger.log("node-pty loaded successfully — terminal sessions available");
|
||||||
|
} catch {
|
||||||
|
this.logger.warn(
|
||||||
|
"node-pty native module not available — terminal sessions will be disabled. " +
|
||||||
|
"Install build tools (python3, make, g++) and rebuild node-pty to enable."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create a new PTY session for the given workspace and socket.
|
* Create a new PTY session for the given workspace and socket.
|
||||||
* Wires PTY onData -> emit terminal:output and onExit -> emit terminal:exit.
|
* Wires PTY onData -> emit terminal:output and onExit -> emit terminal:exit.
|
||||||
*
|
*
|
||||||
* @throws Error if workspace session limit is exceeded
|
* @throws Error if workspace session limit is exceeded or node-pty is unavailable
|
||||||
*/
|
*/
|
||||||
createSession(socket: Socket, options: CreateSessionOptions): SessionCreatedResult {
|
createSession(socket: Socket, options: CreateSessionOptions): SessionCreatedResult {
|
||||||
|
if (!pty) {
|
||||||
|
throw new Error("Terminal sessions are unavailable: node-pty native module failed to load");
|
||||||
|
}
|
||||||
const { workspaceId, name, cwd, socketId } = options;
|
const { workspaceId, name, cwd, socketId } = options;
|
||||||
const cols = options.cols ?? DEFAULT_COLS;
|
const cols = options.cols ?? DEFAULT_COLS;
|
||||||
const rows = options.rows ?? DEFAULT_ROWS;
|
const rows = options.rows ?? DEFAULT_ROWS;
|
||||||
|
|||||||
Reference in New Issue
Block a user