Skip to content

Commit 41a772e

Browse files
committed
feat(apps): harden local execution — serialization, Source, edge cases
Serializes local backend-function executions via a promise-chain queue, since @datadog/action-catalog and @datadog/apps-backend both register runtime context via a shared, module-level setter that isn't safe under concurrent in-process execution. Also populates $.Source with a synthetic local-dev identity (deferred from Milestone 0), and adds edge-case coverage: non-serializable results, a top-level module throw, and a real concurrent-execution test against a genuine @datadog/apps-backend typed import confirming no cross-execution state leakage.
1 parent 7b74053 commit 41a772e

2 files changed

Lines changed: 213 additions & 7 deletions

File tree

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

Lines changed: 143 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ function loadModuleReturning(exports: Record<string, unknown>): LoadModule {
3434
};
3535
}
3636

37+
const ORDER_MARKER = '__ddLocalExecutionTestOrder';
38+
3739
describe('local-execution — executeScriptLocally', () => {
3840
test('Should run a simple function in-process and return its result', async () => {
3941
const result = await executeScriptLocally(
@@ -77,6 +79,18 @@ describe('local-execution — executeScriptLocally', () => {
7779
).rejects.toThrow(`"example" is not a function exported from ${func.absolutePath}`);
7880
});
7981

82+
test('Should reject when loadModule itself rejects, same as a native-module load failure would', async () => {
83+
// Simulates e.g. a native addon failing to load at require()/import
84+
// time, rather than a customer function throwing during its own
85+
// logic — the failure happens before the function is ever reached.
86+
const loadModule: LoadModule = async () => {
87+
throw new Error('cannot find native module');
88+
};
89+
await expect(
90+
executeScriptLocally(func, [], stubExecuteAction, loadModule, mockLogger),
91+
).rejects.toThrow('cannot find native module');
92+
});
93+
8094
test('Should resolve a $.Actions.foo.bar(...) call through the injected executeAction, including connectionId', async () => {
8195
const executeAction = jest.fn().mockResolvedValue({ ok: true });
8296
const result = await executeScriptLocally(
@@ -156,7 +170,20 @@ describe('local-execution — executeScriptLocally', () => {
156170
).rejects.toThrow(/timed out after 50ms/);
157171
});
158172

159-
test('Should never expose an auth token via globalThis', async () => {
173+
test('Should never expose an auth token to the customer module — only backendFunctionArgs, Actions, and Source are visible on globalThis.$', async () => {
174+
const result = await executeScriptLocally(
175+
func,
176+
[],
177+
stubExecuteAction,
178+
loadModuleReturning({
179+
example: () => Object.keys((globalThis as Record<string, any>).$).sort(),
180+
}),
181+
mockLogger,
182+
);
183+
expect(result).toEqual({ data: ['Actions', 'Source', 'backendFunctionArgs'] });
184+
});
185+
186+
test('Should never expose an auth token via globalThis either', async () => {
160187
const result = await executeScriptLocally(
161188
func,
162189
[],
@@ -245,29 +272,139 @@ describe('local-execution — executeScriptLocally', () => {
245272
});
246273
});
247274

275+
describe('non-serializable results', () => {
276+
test('Should reject with a clear, attributed error when the result has a circular reference', async () => {
277+
await expect(
278+
executeScriptLocally(
279+
func,
280+
[],
281+
stubExecuteAction,
282+
loadModuleReturning({
283+
example: () => {
284+
const o: Record<string, unknown> = {};
285+
o.self = o;
286+
return o;
287+
},
288+
}),
289+
mockLogger,
290+
),
291+
).rejects.toThrow(/example.*can't be serialized to JSON/);
292+
});
293+
294+
test('Should reject with a clear, attributed error when the result contains a BigInt', async () => {
295+
await expect(
296+
executeScriptLocally(
297+
func,
298+
[],
299+
stubExecuteAction,
300+
loadModuleReturning({ example: () => BigInt(10) }),
301+
mockLogger,
302+
),
303+
).rejects.toThrow(/example.*can't be serialized to JSON/);
304+
});
305+
306+
test('Should reject with a clear, attributed error when the result is a bare function (silently dropped by JSON.stringify)', async () => {
307+
await expect(
308+
executeScriptLocally(
309+
func,
310+
[],
311+
stubExecuteAction,
312+
loadModuleReturning({ example: () => function notSerializable() {} }),
313+
mockLogger,
314+
),
315+
).rejects.toThrow(/example.*JSON.stringify silently drops/);
316+
});
317+
318+
test('Should allow an explicit undefined result through unchanged', async () => {
319+
const result = await executeScriptLocally(
320+
func,
321+
[],
322+
stubExecuteAction,
323+
loadModuleReturning({ example: () => undefined }),
324+
mockLogger,
325+
);
326+
expect(result).toEqual({ data: undefined });
327+
});
328+
});
329+
248330
describe('serialization of concurrent executions', () => {
249-
function delayedResult<T>(label: T, delayMs: number): () => Promise<T> {
250-
return () => new Promise((resolve) => setTimeout(() => resolve(label), delayMs));
331+
beforeEach(() => {
332+
delete (globalThis as Record<string, unknown>)[ORDER_MARKER];
333+
});
334+
335+
function recordingOrder(label: string, delayMs: number): () => Promise<string> {
336+
return async () => {
337+
const marker =
338+
((globalThis as Record<string, unknown>)[ORDER_MARKER] as string[]) ?? [];
339+
(globalThis as Record<string, unknown>)[ORDER_MARKER] = marker;
340+
marker.push(`start-${label}`);
341+
await new Promise((r) => setTimeout(r, delayMs));
342+
marker.push(`end-${label}`);
343+
return label;
344+
};
251345
}
252346

253-
test("Should allow two independent calls to run without cross-contaminating each other's result", async () => {
347+
test('Should never interleave two concurrent executions — the second never starts until the first fully finishes', async () => {
254348
const [resultA, resultB] = await Promise.all([
255349
executeScriptLocally(
256350
func,
257351
[],
258352
stubExecuteAction,
259-
loadModuleReturning({ example: delayedResult('A', 20) }),
353+
loadModuleReturning({ example: recordingOrder('A', 20) }),
260354
mockLogger,
261355
),
262356
executeScriptLocally(
263357
func,
264358
[],
265359
stubExecuteAction,
266-
loadModuleReturning({ example: delayedResult('B', 0) }),
360+
loadModuleReturning({ example: recordingOrder('B', 0) }),
267361
mockLogger,
268362
),
269363
]);
364+
270365
expect([resultA, resultB]).toEqual([{ data: 'A' }, { data: 'B' }]);
366+
const order = (globalThis as Record<string, unknown>)[ORDER_MARKER];
367+
// Whichever call the queue happened to run first, its start/end
368+
// pair must be adjacent — never interrupted by the other call's
369+
// start. A real race (no queueing) would produce
370+
// ['start-A', 'start-B', 'end-B', 'end-A'] here, since B's 0ms
371+
// delay would let it finish first if both started immediately.
372+
expect(order).toEqual([
373+
expect.stringMatching(/^start-/),
374+
expect.stringMatching(/^end-/),
375+
expect.stringMatching(/^start-/),
376+
expect.stringMatching(/^end-/),
377+
]);
378+
expect((order as string[])[0].slice('start-'.length)).toEqual(
379+
(order as string[])[1].slice('end-'.length),
380+
);
381+
expect((order as string[])[2].slice('start-'.length)).toEqual(
382+
(order as string[])[3].slice('end-'.length),
383+
);
384+
});
385+
386+
test('Should still run the next queued execution after an earlier one rejects', async () => {
387+
const first = executeScriptLocally(
388+
func,
389+
[],
390+
stubExecuteAction,
391+
loadModuleReturning({
392+
example: () => {
393+
throw new Error('first fails');
394+
},
395+
}),
396+
mockLogger,
397+
);
398+
const second = executeScriptLocally(
399+
func,
400+
[],
401+
stubExecuteAction,
402+
loadModuleReturning({ example: () => 2 }),
403+
mockLogger,
404+
);
405+
406+
await expect(first).rejects.toThrow('first fails');
407+
await expect(second).resolves.toEqual({ data: 2 });
271408
});
272409
});
273410
});

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

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,33 @@ const LOCAL_DEV_SOURCE = {
7070
runAsUser: { id: 'local-dev', orgId: 'local-dev-org' },
7171
};
7272

73+
/**
74+
* Local backend-function executions are serialized, never run concurrently.
75+
* `@datadog/action-catalog`'s `setExecuteActionImplementation` and
76+
* `@datadog/apps-backend`'s `setBackend` both register runtime context via a
77+
* shared, module-level setter — safe under production's model (a fresh Deno
78+
* subprocess per execution), unsafe under ours (one long-lived Node process
79+
* for every local execution). A second concurrent execution's registration
80+
* would silently redirect the first's still-in-flight typed-import calls to
81+
* the wrong `$.Actions`/user identity, with no error at all. See the RFC's
82+
* Decisions and Trade-Offs for the full reasoning.
83+
*
84+
* Implementation: a simple promise-chain mutex. `queueTail` always resolves
85+
* (errors are swallowed via `.catch(() => {})` before being chained) so a
86+
* rejected execution never wedges the queue for whatever runs after it; the
87+
* real rejection is still preserved and returned to that call's own caller.
88+
*/
89+
let queueTail: Promise<unknown> = Promise.resolve();
90+
91+
function enqueue<T>(run: () => Promise<T>): Promise<T> {
92+
const result = queueTail.then(run);
93+
queueTail = result.then(
94+
() => undefined,
95+
() => undefined,
96+
);
97+
return result;
98+
}
99+
73100
/**
74101
* Build the $.Actions Proxy. Resolves any nested property path (e.g.
75102
* $.Actions.slack.chat.postMessage) to a callable that invokes
@@ -164,13 +191,44 @@ async function registerBackendRuntimeIfInstalled(
164191
setBackend(buildRuntimeFromJsFunctionWithActions($));
165192
}
166193

194+
/**
195+
* Backend functions eventually return through `ExecuteActionResponse`, which
196+
* is serialized to JSON over HTTP. Catch a non-JSON-serializable result here,
197+
* with a clear, attributed error, rather than let it surface later as an
198+
* opaque `JSON.stringify` failure (or silently drop data) further down the
199+
* response pipeline. Covers two distinct failure shapes: `JSON.stringify`
200+
* throwing outright (a circular reference, a `BigInt`) and `JSON.stringify`
201+
* silently returning `undefined` for a value that wasn't actually `undefined`
202+
* (a bare function or `Symbol` at the top level).
203+
*/
204+
function assertJsonSerializable(result: unknown, func: BackendFunction): unknown {
205+
let serialized: string | undefined;
206+
try {
207+
serialized = JSON.stringify(result);
208+
} catch (err) {
209+
throw new Error(
210+
`Local execution of "${func.name}" returned a value that can't be serialized to JSON: ${
211+
err instanceof Error ? err.message : String(err)
212+
}`,
213+
);
214+
}
215+
if (serialized === undefined && result !== undefined) {
216+
throw new Error(
217+
`Local execution of "${func.name}" returned a ${typeof result} value, which JSON.stringify silently drops instead of serializing — return a plain JSON-compatible value instead.`,
218+
);
219+
}
220+
return result;
221+
}
222+
167223
/**
168224
* Execute a backend function in-process by importing its real file directly
169225
* — no bundling, no generated wrapper module. `globalThis.$` and the
170226
* action-catalog/apps-backend registrations above stand in for what the
171227
* removed `main($)` wrapper used to do textually; everything else about the
172228
* call is just invoking the customer's exported function with its own real
173229
* arguments.
230+
*
231+
* Serialized via `enqueue` — see its own doc comment for why.
174232
*/
175233
export async function executeScriptLocally(
176234
func: BackendFunction,
@@ -179,6 +237,17 @@ export async function executeScriptLocally(
179237
loadModule: LoadModule,
180238
log: Logger,
181239
timeoutMs: number = DEFAULT_TIMEOUT_MS,
240+
): Promise<BackendOutputs> {
241+
return enqueue(() => runScriptLocally(func, args, executeAction, loadModule, log, timeoutMs));
242+
}
243+
244+
async function runScriptLocally(
245+
func: BackendFunction,
246+
args: unknown[],
247+
executeAction: ExecuteAction,
248+
loadModule: LoadModule,
249+
log: Logger,
250+
timeoutMs: number,
182251
): Promise<BackendOutputs> {
183252
log.debug(`Executing "${func.name}" in-process with args=${JSON.stringify(args)}`);
184253

@@ -202,7 +271,7 @@ export async function executeScriptLocally(
202271
}
203272

204273
const result = await fn(...args);
205-
return { data: result };
274+
return { data: assertJsonSerializable(result, func) };
206275
};
207276

208277
let timer: ReturnType<typeof setTimeout> | undefined;

0 commit comments

Comments
 (0)