-
Notifications
You must be signed in to change notification settings - Fork 256
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
channels-1094 #2024
channels-1094 #2024
Changes from 5 commits
28db0aa
cf0b33d
5496895
b792860
4899aeb
8ee70f6
b478bf1
255dba8
837380f
a616a3f
e758ce1
631df68
3244334
7ddc631
522a103
2d5c75f
b8a48fd
3221f34
761b0b9
f5a2997
6f55f20
26faeb8
f7cc09c
1a7fcab
e6d18de
5c86514
7982eb9
47e6208
121afa9
605cd2f
fcb020b
83a201b
72217d7
3eb5fda
516921b
4ea3d45
779c533
4572060
a54ae36
e8e52e6
5e59f18
b22ecf5
cccddf9
7ef8179
8b77355
5357d3f
a9d8806
d970343
f960e80
745cf43
1aa9695
0adc2ac
8ce1549
c1d40d2
0d450e2
90ee709
04c4575
c76c722
3df6a9b
79518a7
2bc39ce
c0a27d4
c7b272e
42da15a
591c5b9
f60d4e2
41c9de1
ad7298c
e0e4c4c
5222383
133d08a
b6ada52
55225b0
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 |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { StateContext, Logger, StatsContext, TransactionContext, DataFeedCache, ActionHookType } from './index' | ||
import { StateContext, Logger, StatsContext, TransactionContext, EngageDestinationCache, ActionHookType } from './index' | ||
import type { RequestOptions } from '../request-client' | ||
import type { JSONObject } from '../json-object' | ||
import { AuthTokens } from './parse-settings' | ||
|
@@ -20,7 +20,9 @@ export interface ExecuteInput< | |
Settings, | ||
Payload, | ||
AudienceSettings = unknown, | ||
/* eslint-disable-next-line @typescript-eslint/no-explicit-any -- Expected any. */ | ||
ActionHookInputs = any, | ||
/* eslint-disable-next-line @typescript-eslint/no-explicit-any -- Expected any. */ | ||
ActionHookOutputs = any | ||
> { | ||
/** The subscription mapping definition */ | ||
|
@@ -39,15 +41,17 @@ export interface ExecuteInput< | |
page?: string | ||
/** The data needed in OAuth requests */ | ||
readonly auth?: AuthTokens | ||
/** | ||
* The features available in the request based on the customer's sourceID; | ||
* `features`,`stats`, `logger` , `transactionContext` and `stateContext` are for internal Twilio/Segment use only. | ||
*/ | ||
/** Engage internal use only. DO NOT USE. */ | ||
readonly features?: Features | ||
Comment on lines
-55
to
+57
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. Broke this comment apart so that we could get language server support for using each property in the shape of this object. |
||
/** Engage internal use only. DO NOT USE. */ | ||
readonly statsContext?: StatsContext | ||
/** Engage internal use only. DO NOT USE. */ | ||
readonly logger?: Logger | ||
readonly dataFeedCache?: DataFeedCache | ||
/** Engage internal use only. DO NOT USE. */ | ||
readonly engageDestinationCache?: EngageDestinationCache | ||
/** Engage internal use only. DO NOT USE. */ | ||
readonly transactionContext?: TransactionContext | ||
/** Engage internal use only. DO NOT USE. */ | ||
readonly stateContext?: StateContext | ||
} | ||
|
||
|
cogwizzle marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,9 +91,86 @@ export abstract class MessageSendPerformer< | |
TSettings extends MessageSettingsBase, | ||
TPayload extends MessagePayloadBase | ||
> extends EngageActionPerformer<TSettings, TPayload> { | ||
/** | ||
* Cache the response from the server based on messageId and recipientId. | ||
* Only messages with valid messageIds, recipientIds, and successful or non | ||
* retryable errors are cacheable. All other errors will be ignored. | ||
* | ||
* @param response The response from the server. | ||
* @param messageId The messageId of the message. | ||
* @param recipientId The recipientId of the message. | ||
* @returns The response from the cache. | ||
*/ | ||
@track() | ||
async cacheResponse(response: Response, messageId?: string, recipientId?: string) { | ||
if (!messageId || !recipientId || !this.engageDestinationCache) { | ||
return | ||
} | ||
// Check if the response is successful. | ||
if (response.ok) { | ||
const body = await response.text() | ||
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. I might be wrong but reading response.text() here will close the stream and will fail next time response body is tried to be read (during actual logic of processing response), so you might want to check it and if it's the case you may use response.clone() or pass here the actual value to be cached, instead of response object 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. Since I'm swapping to use the status code I don't think we need to worry about this. |
||
await this.engageDestinationCache.setByKey(messageId + recipientId, body) | ||
cogwizzle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// If the response is not retryable, cache the error. | ||
} else if (!isRetryableError(response)) { | ||
const error = await response.text() | ||
await this.engageDestinationCache.setByKey(messageId + recipientId, error) | ||
} | ||
} | ||
|
||
/** | ||
* Read the response from the cache based on messageId and recipientId. | ||
* | ||
* @param messageId The messageId of the message. | ||
* @param recipientId The recipientId of the message. | ||
* @returns The response from the cache. | ||
*/ | ||
@track() | ||
async readCache(messageId?: string, recipientId?: string) { | ||
if (!messageId || !recipientId || !this.engageDestinationCache) { | ||
return | ||
} | ||
return this.engageDestinationCache.getByKey(messageId + recipientId) | ||
} | ||
|
||
@track() | ||
cogwizzle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
async doPerform() { | ||
/** | ||
* Internal function to process a recepient. | ||
* | ||
* If a cache exists, it will check to see if the messageId, and recipientId | ||
* has already been attempted. The response will be stored if the error | ||
* returned from the server is a non retryable error or if the response is | ||
* successful. | ||
* | ||
* @param recepient The recepient to process. | ||
* @returns The response from the server. | ||
*/ | ||
const processRecipient = async (recepient: ExtId<TPayload>) => { | ||
const messageId = (this.executeInput as any)['rawData']?.messageId | ||
const recipientId = recepient.id | ||
let cachedResponse | ||
try { | ||
cachedResponse = await this.readCache(messageId, recipientId) | ||
if (cachedResponse) { | ||
return cachedResponse | ||
} | ||
} catch (error) { | ||
this.logError(`Failed to read cache for messageId: ${messageId}, recipientId: ${recipientId}`, error) | ||
} | ||
|
||
const response: Response = await this.sendToRecepient(recepient) | ||
|
||
// This is a non blocking promise. | ||
// Let the cacheResponse throw errors if it fails. | ||
this.cacheResponse(response, messageId, recipientId).catch((error) => { | ||
this.logError(`Failed to cache response for messageId: ${messageId}, recipientId: ${recipientId}`, error) | ||
}) | ||
|
||
return response | ||
} | ||
|
||
// sending messages and collecting results, including exceptions | ||
const res = this.forAllRecepients((recepient) => this.sendToRecepient(recepient)) | ||
const res = this.forAllRecepients(processRecipient) | ||
|
||
if (this.executeInput.features) { | ||
this.logInfo('Feature flags:' + JSON.stringify(this.executeInput.features)) | ||
|
@@ -238,6 +315,12 @@ export abstract class MessageSendPerformer< | |
: [] | ||
} | ||
|
||
/** | ||
* Send message to recipient. | ||
* | ||
* @param recepient The recipient to send the message to. | ||
* @returns The response from the server. | ||
*/ | ||
abstract sendToRecepient(recepient: ExtId<TPayload>): any | ||
|
||
/** | ||
|
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.
Broke this comment apart so that we could get language server support for using each property in the shape of this object.