-
Notifications
You must be signed in to change notification settings - Fork 8
[APPS] Copy apps-function-query runtime into apps plugin #322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
929592a
41540bf
ac028e8
d972daf
05977d5
4e238ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2019-Present Datadog, Inc. | ||
|
|
||
| // Only the fetch (dev-server) transport is exercised here. The iframe | ||
| // postMessage transport requires a DOM (`window`, `MessageEvent`) that this | ||
| // repo's node-only jest harness doesn't provide — adding jsdom collides with | ||
| // the shared `setupAfterEnv.ts` (nock → TextEncoder). postMessage coverage | ||
| // lives with the original tests in web-ui's @datadog/apps-function-query | ||
| // until a DOM-enabled harness is introduced. | ||
|
|
||
| import { executeBackendFunction } from './execute-backend-function'; | ||
| import { BackendFunctionError } from './types'; | ||
|
|
||
| describe('executeBackendFunction', () => { | ||
| let originalFetch: typeof fetch; | ||
|
|
||
| beforeEach(() => { | ||
| originalFetch = global.fetch; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| global.fetch = originalFetch; | ||
| }); | ||
|
|
||
| it('should successfully execute a backend function', async () => { | ||
| const mockResponse = { success: true, result: { data: { sum: 12 } } }; | ||
| global.fetch = jest.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: async () => mockResponse, | ||
| }); | ||
|
|
||
| const result = await executeBackendFunction<{ sum: number }>('testWithImport', [5, 7]); | ||
|
|
||
| expect(result).toEqual({ sum: 12 }); | ||
| expect(global.fetch).toHaveBeenCalledWith('/__dd/executeAction', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| functionName: 'testWithImport', | ||
| args: [5, 7], | ||
| }), | ||
| }); | ||
| }); | ||
|
|
||
| it('should throw BackendFunctionError on network error', async () => { | ||
| global.fetch = jest.fn().mockRejectedValue(new Error('Network failed')); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| BackendFunctionError, | ||
| ); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| 'Network error while executing backend function "testFunction"', | ||
| ); | ||
| }); | ||
|
|
||
| it('should throw BackendFunctionError on non-ok response', async () => { | ||
| global.fetch = jest.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 500, | ||
| text: async () => 'Internal Server Error', | ||
| }); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| BackendFunctionError, | ||
| ); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| 'Backend function "testFunction" failed with status 500', | ||
| ); | ||
| }); | ||
|
|
||
| it('should throw BackendFunctionError when response contains error field', async () => { | ||
| const mockResponse = { | ||
| error: 'Function execution failed', | ||
| data: null, | ||
| }; | ||
| global.fetch = jest.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: async () => mockResponse, | ||
| }); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| BackendFunctionError, | ||
| ); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| 'Backend function "testFunction" returned an error', | ||
| ); | ||
| }); | ||
|
|
||
| it('should throw BackendFunctionError on invalid JSON response', async () => { | ||
| global.fetch = jest.fn().mockResolvedValue({ | ||
| ok: true, | ||
| status: 200, | ||
| json: async () => { | ||
| throw new Error('Invalid JSON'); | ||
| }, | ||
| }); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| BackendFunctionError, | ||
| ); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toThrow( | ||
| 'Failed to parse response from backend function', | ||
| ); | ||
| }); | ||
|
|
||
| it('should include statusCode in error when available', async () => { | ||
| global.fetch = jest.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 404, | ||
| text: async () => 'Not Found', | ||
| }); | ||
|
|
||
| await expect(executeBackendFunction('testFunction', [])).rejects.toMatchObject({ | ||
| name: 'BackendFunctionError', | ||
| functionName: 'testFunction', | ||
| statusCode: 404, | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2019-Present Datadog, Inc. | ||
|
|
||
| /* eslint-env browser */ | ||
|
|
||
| import { devServerTransport } from './transports/dev-server-transport'; | ||
| import { postMessageTransport } from './transports/post-message-transport/post-message-transport'; | ||
| import type { BackendFunctionTransport } from './types'; | ||
|
|
||
| function isInIframe(): boolean { | ||
| try { | ||
| return typeof window !== 'undefined' && window.parent !== window; | ||
| } catch { | ||
| // Accessing window.parent can throw if cross-origin | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| function resolveTransport(): BackendFunctionTransport { | ||
| if (isInIframe()) { | ||
| return postMessageTransport; | ||
| } | ||
| return devServerTransport; | ||
| } | ||
|
|
||
| /** | ||
| * Executes a backend function by name with the provided arguments. | ||
| * | ||
| * When running inside an iframe embedded in App Builder, automatically | ||
| * uses postMessage to communicate with the parent window. Otherwise, | ||
| * uses HTTP fetch to the backend endpoint. | ||
| * | ||
| * @param functionName - The name of the backend function to execute | ||
| * @param args - Array of arguments to pass to the function | ||
| * @returns Promise that resolves to the function's return value | ||
| * @throws {BackendFunctionError} If the request fails or the function throws an error | ||
| * | ||
| * @example | ||
| * ```typescript | ||
| * const result = await executeBackendFunction<{ sum: number }, [number, number]>( | ||
| * 'testWithImport', | ||
| * [5, 7] | ||
| * ); | ||
| * console.log(result.sum); // 12 | ||
| * ``` | ||
| */ | ||
| export async function executeBackendFunction<TData = unknown, TArgs extends unknown[] = unknown[]>( | ||
| functionName: string, | ||
| args: TArgs, | ||
| ): Promise<TData> { | ||
| const transport = resolveTransport(); | ||
| return transport<TData>(functionName, args); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2019-Present Datadog, Inc. | ||
|
|
||
| import type { | ||
| BackendFunctionTransport, | ||
| ExecuteActionRequest, | ||
| ExecuteActionResponse, | ||
| } from '../types'; | ||
| import { BackendFunctionError } from '../types'; | ||
|
|
||
| const ENDPOINT = '/__dd/executeAction'; | ||
|
|
||
| export const devServerTransport: BackendFunctionTransport = async <TData>( | ||
| functionName: string, | ||
| args: unknown[], | ||
| ): Promise<TData> => { | ||
| const request: ExecuteActionRequest = { | ||
| functionName, | ||
| args, | ||
| }; | ||
|
|
||
| let response: Response; | ||
| try { | ||
| response = await fetch(ENDPOINT, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(request), | ||
| }); | ||
| } catch (error) { | ||
| throw new BackendFunctionError( | ||
| `Network error while executing backend function "${functionName}": ${ | ||
| error instanceof Error ? error.message : String(error) | ||
| }`, | ||
| functionName, | ||
| ); | ||
| } | ||
|
|
||
| if (!response.ok) { | ||
| let errorMessage = `Backend function "${functionName}" failed with status ${response.status}`; | ||
| try { | ||
| const errorBody = await response.text(); | ||
| if (errorBody) { | ||
| errorMessage += `: ${errorBody}`; | ||
| } | ||
| } catch { | ||
| // Ignore errors reading error body | ||
| } | ||
| throw new BackendFunctionError(errorMessage, functionName, response.status); | ||
| } | ||
|
|
||
| let executeActionResponse: ExecuteActionResponse<TData>; | ||
| try { | ||
| executeActionResponse = await response.json(); | ||
| } catch (error) { | ||
| throw new BackendFunctionError( | ||
| `Failed to parse response from backend function "${functionName}": ${ | ||
| error instanceof Error ? error.message : String(error) | ||
| }`, | ||
| functionName, | ||
| response.status, | ||
| ); | ||
| } | ||
|
|
||
| if (!executeActionResponse.success) { | ||
| throw new BackendFunctionError( | ||
| `Backend function "${functionName}" returned an error: ${executeActionResponse.error}`, | ||
| functionName, | ||
| response.status, | ||
| ); | ||
| } | ||
|
|
||
| return executeActionResponse.result.data; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This transport assumes successful responses are shaped as Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Responded in the previous thread. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Responded in the previous thread. |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2019-Present Datadog, Inc. | ||
|
|
||
| /* eslint-env browser */ | ||
|
|
||
| import type { BackendFunctionTransport } from '../../types'; | ||
| import { BackendFunctionError } from '../../types'; | ||
|
|
||
| import type { IframeQueryResponse } from './types'; | ||
|
|
||
| const POSTMESSAGE_TIMEOUT_MS = 120_000; | ||
|
|
||
| let requestCounter = 0; | ||
|
|
||
| function generateRequestId(): string { | ||
| if (typeof crypto !== 'undefined' && crypto.randomUUID) { | ||
| return crypto.randomUUID(); | ||
| } | ||
| requestCounter += 1; | ||
| return `req-${Date.now()}-${requestCounter}`; | ||
| } | ||
|
|
||
| function isQueryResponse(data: unknown, requestId: string): data is IframeQueryResponse { | ||
| return ( | ||
| data !== null && | ||
| typeof data === 'object' && | ||
| 'type' in data && | ||
| data.type === 'app-builder:run-query:response' && | ||
| 'requestId' in data && | ||
| data.requestId === requestId | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Transport for executing backend functions via `postMessage` when the app | ||
| * is hosted inside an iframe (e.g. App Builder preview). Sends a | ||
| * `app-builder:run-query` message to the parent window and listens for a | ||
| * matching `app-builder:run-query:response` reply. Rejects if no response | ||
| * arrives within {@link POSTMESSAGE_TIMEOUT_MS}. | ||
| */ | ||
| export const postMessageTransport: BackendFunctionTransport = <TData>( | ||
| functionName: string, | ||
| args: unknown[], | ||
| ): Promise<TData> => { | ||
| const requestId = generateRequestId(); | ||
|
|
||
| return new Promise<TData>((resolve, reject) => { | ||
| let timeoutId: ReturnType<typeof setTimeout>; | ||
|
|
||
| function cleanup(): void { | ||
| window.removeEventListener('message', handleMessage); | ||
| clearTimeout(timeoutId); | ||
| } | ||
|
|
||
| function handleMessage(event: MessageEvent): void { | ||
| if (!isQueryResponse(event.data, requestId)) { | ||
| return; | ||
| } | ||
|
|
||
| cleanup(); | ||
|
|
||
| const response = event.data as IframeQueryResponse<TData>; | ||
|
|
||
| if (response.success) { | ||
| resolve(response.result.data); | ||
| } else { | ||
| reject( | ||
| new BackendFunctionError( | ||
| response.error ?? `Backend function "${functionName}" failed`, | ||
| functionName, | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| window.addEventListener('message', handleMessage); | ||
|
|
||
| timeoutId = setTimeout(() => { | ||
| cleanup(); | ||
| reject( | ||
| new BackendFunctionError( | ||
| `Backend function "${functionName}" timed out waiting for response`, | ||
| functionName, | ||
| ), | ||
| ); | ||
| }, POSTMESSAGE_TIMEOUT_MS); | ||
|
|
||
| window.parent.postMessage( | ||
| { | ||
| type: 'app-builder:run-query', | ||
| requestId, | ||
| queryName: functionName, | ||
| args, | ||
| }, | ||
| '*', | ||
| ); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.result.dataThe dev-server transport currently assumes a success body shaped like
{ success: true, result: { data: ... } }, but the in-repo/__dd/executeActionhandler returns{ success: true, result }directly (seepackages/plugins/apps/src/vite/dev-server.tsand its test assertion indev-server.test.ts). With this code, non-iframe calls will resolveundefinedfor normal backend-function outputs (or crash ifresultis null), so generated proxies will lose return values once this runtime is wired in.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified E2E on the stacked PR #320 (where this transport is actually wired in via
globalThis.DD_APPS_RUNTIME). The hypothesized mismatch doesn't reproduce: the Datadogpreview-asyncqueries API itself returnsoutputs: { data: <value> }, so the wire shape is already whatdevServerTransport.result.dataexpects. The recommendedresult: { data: result }wrap would have double-nested it.Repro on PR #320, dev server running against staging:
Why static review missed it:
dev-server.tsdeclares its own looseExecuteActionResponsewithresult?: unknown, so thesatisfiesat the response site has no teeth. Anddev-server.test.tsmocks Datadog's API asoutputs: { result: 'hello' }— without thedatalayer the real API adds — so the mock has drifted from reality and wouldn't catch a real shape change either.Smaller real fix, split across the stack:
This PR (#322):
ExecuteActionRequest/ExecuteActionResponse<TData>frombackend/client/types.tsinto a newbackend/protocol.ts(single source of truth, neutral location).transports/dev-server-transport.tsandtransports/post-message-transport/types.tsimports to pull from there.PR #320:
dev-server.ts's local interfaces with imports from../backend/protocol; typeexecuteScriptViaDatadogas returning{ data: unknown }sosatisfies ExecuteActionResponseactually checks the wire contract.dev-server.test.tsmocks to use the realoutputs: { data: ... }shape so future drift IS caught.No wire-behavior change. Closing this as not-reproduced; the type + mock work above prevents recurrence. Resolving #3105276088 and #3105612205 as duplicates.