Skip to content

Commit 780b7c7

Browse files
committed
harden local execution: signal reporting, shutdown cleanup, edge-case coverage
- Report the OS signal (SIGSEGV/SIGKILL/etc.) when a child is killed rather than exiting normally, instead of an opaque "exited with code null" -- distinguishes an OOM kill or native-module crash from a plain crash. - Track live children in a Set; export killAllLocalExecutionChildren() for the dev server to call on its own shutdown so executions in flight don't leave orphaned Node processes behind. - Add regression coverage for behavior that already worked but had no test: non-serializable return values (circular references) produce a clean rejection instead of hanging, and concurrent $.Actions calls within a single execution resolve to their own correct results. Milestone 1 of APPS-2792's local Node execution kickoff plan.
1 parent fbb6773 commit 780b7c7

2 files changed

Lines changed: 175 additions & 20 deletions

File tree

packages/plugins/apps/src/vite/local-execution.integration.test.ts

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ import type { BackendFunction } from '../backend/types';
2323
import { generateDevVirtualEntryContent } from '../backend/virtual-entry';
2424

2525
import { getBaseBackendBuildConfig } from './build-config';
26-
import { executeScriptLocally } from './local-execution';
26+
import {
27+
executeScriptLocally,
28+
getMostRecentlyForkedChildForTest,
29+
killAllLocalExecutionChildren,
30+
} from './local-execution';
2731

2832
const log = getMockLogger();
2933

@@ -135,4 +139,112 @@ describe('executeScriptLocally (real bundle, no mocks)', () => {
135139
'deliberate bug in a real bundled backend function',
136140
);
137141
}, 20_000);
142+
143+
test('returns a clean, actionable error when the backend function returns a non-serializable value (a circular reference), instead of hanging or crashing opaquely', async () => {
144+
const workingDir = getTempWorkingDir(`local-exec-poc-circular-${Date.now()}`);
145+
const sourceCode = `
146+
export async function circular() {
147+
const obj = {};
148+
obj.self = obj;
149+
return obj;
150+
}
151+
`;
152+
const code = await bundleRealBackendFunction(workingDir, 'circular', sourceCode);
153+
154+
const func: BackendFunction = {
155+
relativePath: 'src/circular',
156+
name: 'circular',
157+
absolutePath: `${workingDir}/src/circular.backend.ts`,
158+
allowedConnectionIds: [],
159+
};
160+
161+
await expect(executeScriptLocally(code, func, [], log)).rejects.toThrow(
162+
/circular|serializ/i,
163+
);
164+
}, 20_000);
165+
166+
test('resolves each concurrent $.Actions call within a single execution to its own correct result, not a mixed-up one', async () => {
167+
const workingDir = getTempWorkingDir(`local-exec-poc-concurrent-actions-${Date.now()}`);
168+
const sourceCode = `
169+
export async function concurrentActions() {
170+
const [a, b, c] = await Promise.all([
171+
$.Actions.slack.chat.postMessage({ inputs: { channel: '#a' }, connectionId: 'connection:slack:poc' }),
172+
$.Actions.github.issues.create({ inputs: { title: 'b' }, connectionId: 'connection:slack:poc' }),
173+
$.Actions.jira.jira.createIssue({ inputs: { summary: 'c' }, connectionId: 'connection:slack:poc' }),
174+
]);
175+
return { a, b, c };
176+
}
177+
`;
178+
const code = await bundleRealBackendFunction(workingDir, 'concurrentActions', sourceCode);
179+
180+
const func: BackendFunction = {
181+
relativePath: 'src/concurrentActions',
182+
name: 'concurrentActions',
183+
absolutePath: `${workingDir}/src/concurrentActions.backend.ts`,
184+
allowedConnectionIds: ['connection:slack:poc'],
185+
};
186+
187+
const outputs = await executeScriptLocally(code, func, [], log);
188+
189+
expect(outputs.data).toMatchObject({
190+
a: { stub: true, fqn: 'com.datadoghq.slack.chat.postMessage' },
191+
b: { stub: true, fqn: 'com.datadoghq.github.issues.create' },
192+
c: { stub: true, fqn: 'com.datadoghq.jira.jira.createIssue' },
193+
});
194+
}, 20_000);
195+
196+
test('reports the signal when a child process is killed rather than exiting normally (e.g. an OOM-killed or crashed native module), instead of an opaque "code null" error', async () => {
197+
const workingDir = getTempWorkingDir(`local-exec-poc-signal-${Date.now()}`);
198+
// A function that never resolves -- gives the test time to kill the
199+
// child with a signal before it would otherwise finish or time out.
200+
const sourceCode = `
201+
export async function hangs() {
202+
return new Promise(() => {});
203+
}
204+
`;
205+
const code = await bundleRealBackendFunction(workingDir, 'hangs', sourceCode);
206+
207+
const func: BackendFunction = {
208+
relativePath: 'src/hangs',
209+
name: 'hangs',
210+
absolutePath: `${workingDir}/src/hangs.backend.ts`,
211+
allowedConnectionIds: [],
212+
};
213+
214+
const execution = executeScriptLocally(code, func, [], log, 5_000);
215+
const child = getMostRecentlyForkedChildForTest();
216+
// Give the child a moment to actually start running before killing it,
217+
// so this exercises a real in-flight kill, not a race with fork() itself.
218+
await new Promise((resolveDelay) => setTimeout(resolveDelay, 200));
219+
child?.kill('SIGSEGV');
220+
221+
await expect(execution).rejects.toThrow(/SIGSEGV/);
222+
}, 20_000);
223+
224+
test('killAllLocalExecutionChildren terminates an in-flight child (dev-server shutdown must not leave orphaned processes)', async () => {
225+
const workingDir = getTempWorkingDir(`local-exec-poc-shutdown-${Date.now()}`);
226+
const sourceCode = `
227+
export async function hangs() {
228+
return new Promise(() => {});
229+
}
230+
`;
231+
const code = await bundleRealBackendFunction(workingDir, 'hangs', sourceCode);
232+
233+
const func: BackendFunction = {
234+
relativePath: 'src/hangs',
235+
name: 'hangs',
236+
absolutePath: `${workingDir}/src/hangs.backend.ts`,
237+
allowedConnectionIds: [],
238+
};
239+
240+
const execution = executeScriptLocally(code, func, [], log, 5_000);
241+
const child = getMostRecentlyForkedChildForTest();
242+
await new Promise((resolveDelay) => setTimeout(resolveDelay, 200));
243+
244+
expect(child?.killed).toBe(false);
245+
killAllLocalExecutionChildren();
246+
247+
await expect(execution).rejects.toThrow();
248+
expect(child?.killed).toBe(true);
249+
}, 20_000);
138250
});

packages/plugins/apps/src/vite/local-execution.ts

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,39 @@
1616
*/
1717

1818
import type { Logger } from '@dd/core/types';
19+
import type { ChildProcess } from 'child_process';
1920
import { fork } from 'child_process';
2021
import * as path from 'path';
2122

2223
import type { BackendFunction } from '../backend/types';
2324

2425
type BackendOutputs = { data: unknown };
2526

27+
// Tracks every child currently executing a backend function, so the dev
28+
// server can kill them all on its own shutdown instead of leaving orphaned
29+
// Node processes behind (see killAllLocalExecutionChildren below). A Set
30+
// rather than a single reference because multiple executions can be
31+
// in-flight concurrently -- each gets its own forked child (pooling is
32+
// deliberately deferred, see the design doc's Timeline section).
33+
const liveChildren = new Set<ChildProcess>();
34+
35+
/**
36+
* Kill every backend-function child process currently executing. Call this
37+
* from the dev server's own shutdown handling (SIGINT/SIGTERM/process exit)
38+
* once local execution is wired in -- not yet called anywhere, since this
39+
* file isn't wired into createDevServerMiddleware yet.
40+
*/
41+
export function killAllLocalExecutionChildren(): void {
42+
for (const child of liveChildren) {
43+
child.kill();
44+
}
45+
}
46+
47+
/** Test-only: exposes the most recently forked child so tests can exercise real signal-based kills. */
48+
export function getMostRecentlyForkedChildForTest(): ChildProcess | undefined {
49+
return Array.from(liveChildren).at(-1);
50+
}
51+
2652
interface ActionRequestMessage {
2753
type: 'action-request';
2854
id: number;
@@ -73,16 +99,25 @@ export function executeScriptLocally(
7399
// `env`, never inherit process.env wholesale -- see the design doc's
74100
// Secrets Handling section (never hand secrets to the child).
75101
const child = fork(LOCAL_EXEC_CHILD_SCRIPT, [], { stdio: 'inherit', env: {} });
102+
liveChildren.add(child);
76103
let settled = false;
77104

105+
const settle = (fn: () => void) => {
106+
if (settled) {
107+
return;
108+
}
109+
settled = true;
110+
liveChildren.delete(child);
111+
fn();
112+
};
113+
78114
const timer = setTimeout(() => {
79-
if (!settled) {
80-
settled = true;
115+
settle(() => {
81116
child.kill();
82117
reject(
83118
new Error(`Local execution of "${func.name}" timed out after ${timeoutMs}ms`),
84119
);
85-
}
120+
});
86121
}, timeoutMs);
87122

88123
child.on('message', (msg: ChildMessage) => {
@@ -105,38 +140,46 @@ export function executeScriptLocally(
105140
return;
106141
}
107142

108-
if (msg.type === 'result' && !settled) {
109-
settled = true;
110-
clearTimeout(timer);
111-
resolve({ data: msg.result });
143+
if (msg.type === 'result') {
144+
settle(() => {
145+
clearTimeout(timer);
146+
resolve({ data: msg.result });
147+
});
112148
return;
113149
}
114150

115-
if (msg.type === 'error' && !settled) {
116-
settled = true;
117-
clearTimeout(timer);
118-
reject(new Error(msg.error));
151+
if (msg.type === 'error') {
152+
settle(() => {
153+
clearTimeout(timer);
154+
reject(new Error(msg.error));
155+
});
119156
}
120157
});
121158

122-
child.on('exit', (code) => {
123-
if (!settled) {
124-
settled = true;
159+
child.on('exit', (code, signal) => {
160+
settle(() => {
125161
clearTimeout(timer);
162+
// A signal (not a plain exit code) means the OS terminated the
163+
// process directly -- most commonly an OOM kill (SIGKILL) or a
164+
// native-module crash (SIGSEGV/SIGABRT). Report it explicitly:
165+
// "exited with code null" is meaningless to a developer trying
166+
// to tell an OOM apart from a native-module crash.
167+
const cause = signal
168+
? `was killed by signal ${signal}`
169+
: `exited with code ${code}`;
126170
reject(
127171
new Error(
128-
`Local execution of "${func.name}" exited with code ${code} before reporting a result`,
172+
`Local execution of "${func.name}" ${cause} before reporting a result`,
129173
),
130174
);
131-
}
175+
});
132176
});
133177

134178
child.on('error', (err) => {
135-
if (!settled) {
136-
settled = true;
179+
settle(() => {
137180
clearTimeout(timer);
138181
reject(err);
139-
}
182+
});
140183
});
141184

142185
child.send({ type: 'execute', scriptBody, backendFunctionArgs: args });

0 commit comments

Comments
 (0)