Let server route resolvers answer the caller synchronously (#23233)
## Problem
A server route resolver can only return a dispatch target (`{
workspaceId, targetLogicFunctionUniversalIdentifier, payload }`), and
`ServerRouteTriggerService` always acks `202 {queued:true}`. The target
function runs off the queue, after the response has been sent, so its
return value can never reach the caller.
That makes it impossible to integrate a provider whose webhook URL has
to be proven with a handshake on the same response. Slack's Events API
is the case that surfaced it: `url_verification` sends `{ type,
challenge }` and will not accept the Request URL unless the challenge
comes back on that POST.
## Change
A resolver may now return a `Response` (the existing
`LogicFunctionHttpResponse`) instead of a dispatch target. The route
sends it as-is via `buildRouteTriggerResponse` and enqueues nothing.
- Reuses the marker and builder that HTTP route triggers already use, so
there is no new response shape.
- Dispatch results behave exactly as before; the resolver error path is
unchanged, just hoisted out of `parseResolverResult` so it runs before
the branch.
- SDK: `ServerRouteResolverResult` becomes `ServerRouteDispatchResult |
LogicFunctionHttpResponse`.
Additive: a resolver that returns a dispatch target sees no behavior
change. Previously, returning this shape threw
`RESOLVER_INVALID_RESULT`.
## Testing
`server-route-trigger.service.spec.ts` gains a case asserting the
resolver's response is sent verbatim and nothing is enqueued. 15/15
pass.
## Context
Split out of #22984 (Slack conversational assistant), which needs this
to complete the Slack Events URL verification. That PR depends on this
one merging first.
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/23233?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
This commit is contained in:
+28
-6
@@ -1,4 +1,5 @@
|
||||
import { type Request } from 'express';
|
||||
import { LOGIC_FUNCTION_HTTP_RESPONSE_MARKER } from 'twenty-shared/types';
|
||||
import { type Repository } from 'typeorm';
|
||||
|
||||
import { type LogicFunctionExecuteResult } from 'src/engine/core-modules/logic-function/logic-function-drivers/interfaces/logic-function-driver.interface';
|
||||
@@ -18,8 +19,9 @@ import {
|
||||
} from 'src/engine/metadata-modules/logic-function/logic-function.exception';
|
||||
import { LogicFunctionExecutionStatus } from 'src/engine/metadata-modules/logic-function/dtos/logic-function-execution-result.dto';
|
||||
|
||||
const RESOLVER_UID = 'resolver-uid';
|
||||
const TARGET_UID = 'target-uid';
|
||||
const RESOLVER_UID = 'b3c2f0a1-7d4e-4c9a-9f2b-2e1d6a4c8e10';
|
||||
const TARGET_UID = 'c4e2a9b1-7d4e-4c9a-9f2b-2e1d6a4c8e10';
|
||||
const TARGET_WORKSPACE_ID = '20202020-1c25-4d02-bf25-6aeccf7ea419';
|
||||
|
||||
const buildExecuteResult = (
|
||||
data: object | null,
|
||||
@@ -116,7 +118,7 @@ describe('ServerRouteTriggerService', () => {
|
||||
logicFunctionExecutorService = {
|
||||
execute: jest.fn().mockResolvedValueOnce(
|
||||
buildExecuteResult({
|
||||
workspaceId: 'target-ws',
|
||||
workspaceId: TARGET_WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UID,
|
||||
payload: { from: 'resolver' },
|
||||
}),
|
||||
@@ -148,7 +150,7 @@ describe('ServerRouteTriggerService', () => {
|
||||
LogicFunctionTriggerJob.name,
|
||||
{
|
||||
logicFunctionId: 'target-id',
|
||||
workspaceId: 'target-ws',
|
||||
workspaceId: TARGET_WORKSPACE_ID,
|
||||
payload: { from: 'resolver' },
|
||||
},
|
||||
{ retryLimit: 3 },
|
||||
@@ -200,7 +202,7 @@ describe('ServerRouteTriggerService', () => {
|
||||
expect.objectContaining({
|
||||
where: expect.objectContaining({
|
||||
universalIdentifier: TARGET_UID,
|
||||
workspaceId: 'target-ws',
|
||||
workspaceId: TARGET_WORKSPACE_ID,
|
||||
application: { applicationRegistrationId: 'reg-1' },
|
||||
}),
|
||||
}),
|
||||
@@ -229,6 +231,26 @@ describe('ServerRouteTriggerService', () => {
|
||||
expect(logicFunctionExecutorService.execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('answers the caller directly and enqueues nothing when the resolver returns an http response', async () => {
|
||||
logicFunctionExecutorService.execute.mockReset();
|
||||
logicFunctionExecutorService.execute.mockResolvedValueOnce(
|
||||
buildExecuteResult({
|
||||
[LOGIC_FUNCTION_HTTP_RESPONSE_MARKER]: true,
|
||||
status: 200,
|
||||
body: { challenge: 'abc123' },
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await handle();
|
||||
|
||||
expect(result).toEqual({
|
||||
statusCode: 200,
|
||||
headers: {},
|
||||
body: { challenge: 'abc123' },
|
||||
});
|
||||
expect(messageQueueService.add).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('throws RESOLVER_INVALID_RESULT when the resolver does not return a workspaceId', async () => {
|
||||
logicFunctionExecutorService.execute.mockReset();
|
||||
logicFunctionExecutorService.execute.mockResolvedValueOnce(
|
||||
@@ -245,7 +267,7 @@ describe('ServerRouteTriggerService', () => {
|
||||
it('throws RESOLVER_INVALID_RESULT when the resolver does not return a targetLogicFunctionUniversalIdentifier', async () => {
|
||||
logicFunctionExecutorService.execute.mockReset();
|
||||
logicFunctionExecutorService.execute.mockResolvedValueOnce(
|
||||
buildExecuteResult({ workspaceId: 'target-ws' }),
|
||||
buildExecuteResult({ workspaceId: TARGET_WORKSPACE_ID }),
|
||||
);
|
||||
|
||||
await expect(handle()).rejects.toMatchObject({
|
||||
|
||||
+24
-50
@@ -1,8 +1,8 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { isString } from '@sniptt/guards';
|
||||
import { Request } from 'express';
|
||||
import { isLogicFunctionHttpResponse } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { Repository } from 'typeorm';
|
||||
|
||||
@@ -19,23 +19,21 @@ import { buildLogicFunctionEvent } from 'src/engine/core-modules/logic-function/
|
||||
import { InjectMessageQueue } from 'src/engine/core-modules/message-queue/decorators/message-queue.decorator';
|
||||
import { MessageQueue } from 'src/engine/core-modules/message-queue/message-queue.constants';
|
||||
import { MessageQueueService } from 'src/engine/core-modules/message-queue/services/message-queue.service';
|
||||
import { type RouteTriggerResponse } from 'src/engine/core-modules/logic-function/logic-function-trigger/triggers/route/utils/route-trigger-response.util';
|
||||
import {
|
||||
buildRouteTriggerResponse,
|
||||
type RouteTriggerResponse,
|
||||
} from 'src/engine/core-modules/logic-function/logic-function-trigger/triggers/route/utils/route-trigger-response.util';
|
||||
import {
|
||||
ServerRouteTriggerException,
|
||||
ServerRouteTriggerExceptionCode,
|
||||
} from 'src/engine/core-modules/server-route-trigger/exceptions/server-route-trigger.exception';
|
||||
import { parseResolverDispatchResultOrThrow } from 'src/engine/core-modules/server-route-trigger/utils/parse-resolver-dispatch-result-or-throw.util';
|
||||
import { LogicFunctionEntity } from 'src/engine/metadata-modules/logic-function/logic-function.entity';
|
||||
import {
|
||||
LogicFunctionException,
|
||||
LogicFunctionExceptionCode,
|
||||
} from 'src/engine/metadata-modules/logic-function/logic-function.exception';
|
||||
|
||||
type ResolverResult = {
|
||||
workspaceId: string;
|
||||
targetLogicFunctionUniversalIdentifier: string;
|
||||
payload?: object;
|
||||
};
|
||||
|
||||
const QUEUED_TARGET_RETRY_LIMIT = 3;
|
||||
|
||||
@Injectable()
|
||||
@@ -99,13 +97,27 @@ export class ServerRouteTriggerService {
|
||||
workspaceId: resolver.workspaceId,
|
||||
payload: event,
|
||||
});
|
||||
const resolved = this.parseResolverResult(resolverResult);
|
||||
|
||||
if (isDefined(resolverResult.error)) {
|
||||
throw new ServerRouteTriggerException(
|
||||
resolverResult.error.errorMessage,
|
||||
ServerRouteTriggerExceptionCode.SERVER_ROUTE_USER_UNCAUGHT_ERROR,
|
||||
);
|
||||
}
|
||||
|
||||
if (isLogicFunctionHttpResponse(resolverResult.data)) {
|
||||
return buildRouteTriggerResponse(resolverResult.data);
|
||||
}
|
||||
|
||||
const dispatchResult = parseResolverDispatchResultOrThrow(
|
||||
resolverResult.data,
|
||||
);
|
||||
|
||||
return await this.enqueueTargetFunction({
|
||||
logicFunctionUniversalIdentifier:
|
||||
resolved.targetLogicFunctionUniversalIdentifier,
|
||||
workspaceId: resolved.workspaceId,
|
||||
payload: resolved.payload ?? event,
|
||||
dispatchResult.targetLogicFunctionUniversalIdentifier,
|
||||
workspaceId: dispatchResult.workspaceId,
|
||||
payload: dispatchResult.payload ?? event,
|
||||
applicationRegistrationId,
|
||||
});
|
||||
}
|
||||
@@ -134,44 +146,6 @@ export class ServerRouteTriggerService {
|
||||
);
|
||||
}
|
||||
|
||||
private parseResolverResult(result: {
|
||||
data: object | null;
|
||||
error?: { errorMessage: string };
|
||||
}): ResolverResult {
|
||||
if (isDefined(result.error)) {
|
||||
throw new ServerRouteTriggerException(
|
||||
result.error.errorMessage,
|
||||
ServerRouteTriggerExceptionCode.SERVER_ROUTE_USER_UNCAUGHT_ERROR,
|
||||
);
|
||||
}
|
||||
|
||||
const data = result.data as {
|
||||
workspaceId?: unknown;
|
||||
targetLogicFunctionUniversalIdentifier?: unknown;
|
||||
payload?: unknown;
|
||||
};
|
||||
|
||||
if (
|
||||
!isString(data?.workspaceId) ||
|
||||
!isString(data?.targetLogicFunctionUniversalIdentifier)
|
||||
) {
|
||||
throw new ServerRouteTriggerException(
|
||||
'Resolver logic function must return { workspaceId: string; targetLogicFunctionUniversalIdentifier: string; payload?: object }',
|
||||
ServerRouteTriggerExceptionCode.RESOLVER_INVALID_RESULT,
|
||||
);
|
||||
}
|
||||
|
||||
return {
|
||||
workspaceId: data.workspaceId,
|
||||
targetLogicFunctionUniversalIdentifier:
|
||||
data.targetLogicFunctionUniversalIdentifier,
|
||||
payload:
|
||||
typeof data.payload === 'object' && data.payload !== null
|
||||
? (data.payload as object)
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
private async enqueueTargetFunction({
|
||||
logicFunctionUniversalIdentifier,
|
||||
workspaceId,
|
||||
|
||||
+93
@@ -0,0 +1,93 @@
|
||||
import { ServerRouteTriggerExceptionCode } from 'src/engine/core-modules/server-route-trigger/exceptions/server-route-trigger.exception';
|
||||
import { parseResolverDispatchResultOrThrow } from 'src/engine/core-modules/server-route-trigger/utils/parse-resolver-dispatch-result-or-throw.util';
|
||||
|
||||
const WORKSPACE_ID = '20202020-1c25-4d02-bf25-6aeccf7ea419';
|
||||
const TARGET_UNIVERSAL_IDENTIFIER = 'c4e2a9b1-7d4e-4c9a-9f2b-2e1d6a4c8e10';
|
||||
|
||||
const expectInvalidResult = (data: unknown) =>
|
||||
expect(() => parseResolverDispatchResultOrThrow(data)).toThrow(
|
||||
expect.objectContaining({
|
||||
code: ServerRouteTriggerExceptionCode.RESOLVER_INVALID_RESULT,
|
||||
}),
|
||||
);
|
||||
|
||||
describe('parseResolverDispatchResultOrThrow', () => {
|
||||
it('should return the dispatch target when the resolver returns one', () => {
|
||||
expect(
|
||||
parseResolverDispatchResultOrThrow({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
}),
|
||||
).toEqual({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
});
|
||||
});
|
||||
|
||||
it('should keep the payload when the resolver transforms it', () => {
|
||||
expect(
|
||||
parseResolverDispatchResultOrThrow({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
payload: { event: 'invoice.paid' },
|
||||
}).payload,
|
||||
).toEqual({ event: 'invoice.paid' });
|
||||
});
|
||||
|
||||
it('should accept any object as a payload', () => {
|
||||
expect(
|
||||
parseResolverDispatchResultOrThrow({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
payload: [{ event: 'invoice.paid' }],
|
||||
}).payload,
|
||||
).toEqual([{ event: 'invoice.paid' }]);
|
||||
});
|
||||
|
||||
it('should drop keys that are not part of the dispatch contract', () => {
|
||||
expect(
|
||||
parseResolverDispatchResultOrThrow({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
applicationRegistrationId: 'smuggled',
|
||||
}),
|
||||
).toEqual({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw RESOLVER_INVALID_RESULT when the workspaceId is missing', () => {
|
||||
expectInvalidResult({
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw RESOLVER_INVALID_RESULT when the target identifier is missing', () => {
|
||||
expectInvalidResult({ workspaceId: WORKSPACE_ID });
|
||||
});
|
||||
|
||||
it('should throw RESOLVER_INVALID_RESULT when an identifier is not a uuid', () => {
|
||||
expectInvalidResult({
|
||||
workspaceId: '',
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
});
|
||||
expectInvalidResult({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: 'handle-invoice-paid',
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw RESOLVER_INVALID_RESULT when the payload is not an object', () => {
|
||||
expectInvalidResult({
|
||||
workspaceId: WORKSPACE_ID,
|
||||
targetLogicFunctionUniversalIdentifier: TARGET_UNIVERSAL_IDENTIFIER,
|
||||
payload: 'not-an-object',
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw RESOLVER_INVALID_RESULT when the resolver returns nothing', () => {
|
||||
expectInvalidResult(null);
|
||||
expectInvalidResult(undefined);
|
||||
});
|
||||
});
|
||||
+29
@@ -0,0 +1,29 @@
|
||||
import { isObject } from '@sniptt/guards';
|
||||
import { type ServerRouteDispatchResult } from 'twenty-shared/application';
|
||||
import { z } from 'zod';
|
||||
|
||||
import {
|
||||
ServerRouteTriggerException,
|
||||
ServerRouteTriggerExceptionCode,
|
||||
} from 'src/engine/core-modules/server-route-trigger/exceptions/server-route-trigger.exception';
|
||||
|
||||
const resolverDispatchResultSchema = z.object({
|
||||
workspaceId: z.uuid(),
|
||||
targetLogicFunctionUniversalIdentifier: z.uuid(),
|
||||
payload: z.custom<object>((value) => isObject(value)).optional(),
|
||||
});
|
||||
|
||||
export const parseResolverDispatchResultOrThrow = (
|
||||
data: unknown,
|
||||
): ServerRouteDispatchResult => {
|
||||
const parsedDispatchResult = resolverDispatchResultSchema.safeParse(data);
|
||||
|
||||
if (!parsedDispatchResult.success) {
|
||||
throw new ServerRouteTriggerException(
|
||||
'Resolver logic function must return either a Response, or { workspaceId: string; targetLogicFunctionUniversalIdentifier: string; payload?: object }',
|
||||
ServerRouteTriggerExceptionCode.RESOLVER_INVALID_RESULT,
|
||||
);
|
||||
}
|
||||
|
||||
return parsedDispatchResult.data;
|
||||
};
|
||||
Reference in New Issue
Block a user