[WIP] Framework level changes to support polling by async actions#3734
[WIP] Framework level changes to support polling by async actions#3734arnav777dev wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Framework-level updates in packages/core to introduce a polling execution path for async actions, including new public types and new Destination/Action entrypoints to invoke partner-defined polling logic.
Changes:
- Added
AsyncPollResponseType(and related result shape) to core destination-kit types and exported it from public entrypoints. - Extended
ActionDefinitionto optionally supportpollFields+pollStatus, and addedAction.executePoll()to validate and execute polling requests. - Added
Destination.executePoll()to route poll calls to the appropriate action.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/core/src/index.ts | Exports the new polling response type from core. |
| packages/core/src/destination-kit/types.ts | Introduces new polling result/response types. |
| packages/core/src/destination-kit/index.ts | Adds Destination.executePoll() and re-exports polling types. |
| packages/core/src/destination-kit/action.ts | Extends action definitions with polling support and adds Action.executePoll(). |
| let audienceSettings = {} as AudienceSettings | ||
| if (event.context?.personas) { | ||
| audienceSettings = event.context?.personas?.audience_settings as AudienceSettings | ||
| } | ||
| const authData = getAuthData(settings as JSONObject) | ||
| return action.executePoll({ |
There was a problem hiding this comment.
Destination.executePoll ignores the optional auth provided in EventInput and always derives auth via getAuthData(settings), which only covers OAuth tokens. This can break non-OAuth schemes and also surprises callers that explicitly pass auth. Prefer using the passed auth when provided, and only fallback to deriving it from settings if it is missing.
| ): Promise<AsyncPollResponseType> { | ||
| const action = this.actions[actionSlug] | ||
| if (!action) { | ||
| throw new IntegrationError(`Action ${actionSlug} not found`, 'NotImplemented', 404) |
There was a problem hiding this comment.
When the action slug is missing, executePoll throws an IntegrationError, but executeBatch/executeDynamicField/executeAction return an empty result for unknown actions. This inconsistency makes the public API harder to use. Consider aligning executePoll behavior with the other entrypoints (e.g., return an empty response or a consistent sentinel) unless there’s a strong reason to throw.
| throw new IntegrationError(`Action ${actionSlug} not found`, 'NotImplemented', 404) | |
| return [] as AsyncPollResponseType |
| async executePoll( | ||
| bundle: ExecuteBundle<Settings, InputData | undefined, AudienceSettings> | ||
| ): Promise<AsyncPollResponseType> { | ||
| if (!this.hasPollSupport || !this.definition.pollStatus) { | ||
| throw new IntegrationError('This action does not support polling.', 'NotImplemented', 501) | ||
| } | ||
|
|
||
| const payload = bundle.data as PollPayload | ||
| // Remove empty values and validate using poll schema (required for polling operations) | ||
| if (!this.pollSchema) { | ||
| throw new IntegrationError('Poll fields must be defined for polling operations.', 'NotImplemented', 501) | ||
| } | ||
| const validationSchema = this.pollSchema | ||
| // Cast to PollPayload as the removeEmptyValues pipeline produces a valid poll payload | ||
| // This represents the PollPayload type defined in the ActionDefinition (e.g., { operationId: string }) | ||
| const pollPayload = removeEmptyValues(payload, validationSchema, true) as PollPayload | ||
| // Validate the resolved payload against the poll schema | ||
| const schemaKey = `${this.destinationName}:${this.definition.title}:poll` | ||
| validateSchema(pollPayload, validationSchema, { | ||
| schemaKey, | ||
| statsContext: bundle.statsContext, | ||
| exempt: ['dynamicAuthSettings'] | ||
| }) | ||
|
|
||
| // Construct the data bundle to send to the poll action | ||
| const dataBundle = { | ||
| rawData: bundle.data, | ||
| rawMapping: bundle.mapping, | ||
| settings: bundle.settings, | ||
| payload: pollPayload, | ||
| auth: bundle.auth, | ||
| features: bundle.features, | ||
| statsContext: bundle.statsContext, | ||
| logger: bundle.logger, | ||
| engageDestinationCache: bundle.engageDestinationCache, | ||
| transactionContext: bundle.transactionContext, | ||
| stateContext: bundle.stateContext, | ||
| audienceSettings: bundle.audienceSettings, | ||
| subscriptionMetadata: bundle.subscriptionMetadata, | ||
| signal: bundle?.signal | ||
| } | ||
|
|
||
| // Construct the request client and perform the poll operation | ||
| const requestClient = this.createRequestClient(dataBundle) | ||
| const pollResponse = await this.definition.pollStatus(requestClient, dataBundle) | ||
|
|
||
| return pollResponse | ||
| } |
There was a problem hiding this comment.
New polling entrypoints (Destination.executePoll / Action.executePoll) add significant framework behavior (schema generation, validation, request execution) but there are no corresponding unit tests alongside the existing destination-kit tests. Add tests that cover: action without pollStatus, missing pollFields, successful poll execution, and invalid poll payload validation.
| async executePoll( | ||
| bundle: ExecuteBundle<Settings, InputData | undefined, AudienceSettings> | ||
| ): Promise<AsyncPollResponseType> { | ||
| if (!this.hasPollSupport || !this.definition.pollStatus) { | ||
| throw new IntegrationError('This action does not support polling.', 'NotImplemented', 501) | ||
| } | ||
|
|
||
| const payload = bundle.data as PollPayload | ||
| // Remove empty values and validate using poll schema (required for polling operations) | ||
| if (!this.pollSchema) { | ||
| throw new IntegrationError('Poll fields must be defined for polling operations.', 'NotImplemented', 501) | ||
| } | ||
| const validationSchema = this.pollSchema | ||
| // Cast to PollPayload as the removeEmptyValues pipeline produces a valid poll payload | ||
| // This represents the PollPayload type defined in the ActionDefinition (e.g., { operationId: string }) | ||
| const pollPayload = removeEmptyValues(payload, validationSchema, true) as PollPayload | ||
| // Validate the resolved payload against the poll schema | ||
| const schemaKey = `${this.destinationName}:${this.definition.title}:poll` | ||
| validateSchema(pollPayload, validationSchema, { | ||
| schemaKey, | ||
| statsContext: bundle.statsContext, | ||
| exempt: ['dynamicAuthSettings'] | ||
| }) |
There was a problem hiding this comment.
executePoll can throw a runtime TypeError when bundle.data is undefined: removeEmptyValues will return undefined, and then validateSchema spreads the object ({ ...(obj as Record<string, unknown>) }), which crashes for undefined. Add an explicit guard (and a clearer IntegrationError/validation error) when the poll input is missing or not an object before calling removeEmptyValues/validateSchema.
| const payload = bundle.data as PollPayload | ||
| // Remove empty values and validate using poll schema (required for polling operations) | ||
| if (!this.pollSchema) { | ||
| throw new IntegrationError('Poll fields must be defined for polling operations.', 'NotImplemented', 501) | ||
| } | ||
| const validationSchema = this.pollSchema | ||
| // Cast to PollPayload as the removeEmptyValues pipeline produces a valid poll payload | ||
| // This represents the PollPayload type defined in the ActionDefinition (e.g., { operationId: string }) | ||
| const pollPayload = removeEmptyValues(payload, validationSchema, true) as PollPayload |
There was a problem hiding this comment.
pollFields are converted into pollSchema, but executePoll never applies the mapping transform step (unlike execute, which uses transform(bundle.mapping, bundle.data, ...)). As a result, polling ignores the provided mapping and effectively requires callers to pass an already-shaped poll payload, which makes pollFields misleading and breaks consistency with other action entrypoints. Consider transforming bundle.mapping + bundle.data into a poll payload (using pollFields) before removeEmptyValues/validation.
| const payload = bundle.data as PollPayload | |
| // Remove empty values and validate using poll schema (required for polling operations) | |
| if (!this.pollSchema) { | |
| throw new IntegrationError('Poll fields must be defined for polling operations.', 'NotImplemented', 501) | |
| } | |
| const validationSchema = this.pollSchema | |
| // Cast to PollPayload as the removeEmptyValues pipeline produces a valid poll payload | |
| // This represents the PollPayload type defined in the ActionDefinition (e.g., { operationId: string }) | |
| const pollPayload = removeEmptyValues(payload, validationSchema, true) as PollPayload | |
| // Remove empty values and validate using poll schema (required for polling operations) | |
| if (!this.pollSchema) { | |
| throw new IntegrationError('Poll fields must be defined for polling operations.', 'NotImplemented', 501) | |
| } | |
| const validationSchema = this.pollSchema | |
| const resolvedPayload = | |
| bundle.mapping && this.definition.pollFields | |
| ? ((await transform(bundle.mapping, bundle.data, this.definition.pollFields)) as PollPayload) | |
| : (bundle.data as PollPayload) | |
| // Cast to PollPayload as the removeEmptyValues pipeline produces a valid poll payload | |
| // This represents the PollPayload type defined in the ActionDefinition (e.g., { operationId: string }) | |
| const pollPayload = removeEmptyValues(resolvedPayload, validationSchema, true) as PollPayload |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (14.28%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3734 +/- ##
==========================================
- Coverage 80.94% 80.84% -0.10%
==========================================
Files 1637 1386 -251
Lines 31963 27929 -4034
Branches 7096 6011 -1085
==========================================
- Hits 25872 22579 -3293
+ Misses 5112 4390 -722
+ Partials 979 960 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
A summary of your pull request, including the what change you're making and why.
Testing
Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.
Security Review
Please ensure sensitive data is properly protected in your integration.
type: 'password'New Destination Checklist
verioning-info.tsfile. example