Skip to content

Commit 6967df9

Browse files
committed
fix(slack/onboard-llmo): fire and forget onboarding process
1 parent 695c46e commit 6967df9

File tree

2 files changed

+77
-11
lines changed

2 files changed

+77
-11
lines changed

src/support/slack/actions/onboard-llmo-modal.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -670,8 +670,7 @@ export function onboardLLMOModal(lambdaContext) {
670670
originalThreadTs,
671671
});
672672

673-
// eslint-disable-next-line max-statements-per-line
674-
await new Promise((resolve) => { setTimeout(resolve, 500); });
673+
// Acknowledge the modal immediately to avoid Slack's 3-second timeout
675674
await ack();
676675

677676
// Create a slack context for the onboarding process
@@ -692,9 +691,14 @@ export function onboardLLMOModal(lambdaContext) {
692691
threadTs: responseThreadTs,
693692
};
694693

695-
await onboardSite({
694+
// Fire-and-forget: Start the onboarding process without awaiting
695+
// This allows the handler to return immediately after ack() while the
696+
// onboarding continues in the background
697+
onboardSite({
696698
brandName, baseURL: brandURL, imsOrgId, deliveryType,
697-
}, lambdaContext, slackContext);
699+
}, lambdaContext, slackContext).catch((error) => {
700+
log.error(`Error in background onboarding for site ${brandURL}:`, error);
701+
});
698702

699703
log.debug(`Onboard LLMO modal processed for user ${user.id}, site ${brandURL}`);
700704
} catch (e) {

test/support/slack/actions/onboard-llmo-modal.test.js

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,7 +1188,62 @@ example-com:
11881188
expect(mockAck).to.have.been.calledOnce;
11891189
});
11901190

1191-
it('should print error message if onboarding throws an error', async () => {
1191+
it('should return immediately after ack without waiting for onboarding to complete', async () => {
1192+
const mockBody = {
1193+
view: {
1194+
state: {
1195+
values: {
1196+
brand_name_input: {
1197+
brand_name: { value: 'Test Brand' },
1198+
},
1199+
ims_org_input: {
1200+
ims_org_id: { value: 'ABC123@AdobeOrg' },
1201+
},
1202+
delivery_type_input: {
1203+
delivery_type: {
1204+
selected_option: { value: 'aem_edge' },
1205+
},
1206+
},
1207+
},
1208+
},
1209+
private_metadata: JSON.stringify({
1210+
originalChannel: 'C1234567890',
1211+
originalThreadTs: '1234567890.123456',
1212+
brandURL: 'https://example.com',
1213+
}),
1214+
},
1215+
user: { id: 'U1234567890' },
1216+
};
1217+
1218+
const mockAck = sandbox.stub();
1219+
const mockClient = {
1220+
chat: {
1221+
postMessage: sandbox.stub().resolves(),
1222+
},
1223+
};
1224+
1225+
// Create a slow onboarding process to verify fire-and-forget behavior
1226+
const lambdaCtx = createDefaultMockLambdaCtx(sandbox);
1227+
const slowOnboardingPromise = new Promise((resolve) => {
1228+
setTimeout(resolve, 1000); // Simulate slow operation
1229+
});
1230+
lambdaCtx.dataAccess.Site.findByBaseURL = sandbox.stub().returns(slowOnboardingPromise);
1231+
1232+
const { onboardLLMOModal } = mockedModule;
1233+
const handler = onboardLLMOModal(lambdaCtx);
1234+
1235+
const startTime = Date.now();
1236+
await handler({ ack: mockAck, body: mockBody, client: mockClient });
1237+
const endTime = Date.now();
1238+
1239+
// Handler should return quickly (well under 1 second), not wait for the slow operation
1240+
expect(endTime - startTime).to.be.lessThan(500);
1241+
expect(mockAck).to.have.been.calledOnce;
1242+
expect(mockAck).to.have.been.calledWith(); // Called without errors
1243+
expect(lambdaCtx.log.debug).to.have.been.calledWith('Onboard LLMO modal processed for user U1234567890, site https://example.com');
1244+
});
1245+
1246+
it('should log error if onboarding throws an error in background', async () => {
11921247
const mockBody = {
11931248
view: {
11941249
state: {
@@ -1224,13 +1279,20 @@ example-com:
12241279

12251280
await handler({ ack: mockAck, body: mockBody, client: mockClient });
12261281

1227-
expect(lambdaCtx.log.error).to.have.been.calledWith('Error handling onboard site modal:', sinon.match.instanceOf(Error));
1228-
expect(mockAck).to.have.been.calledWith({
1229-
response_action: 'errors',
1230-
errors: {
1231-
brand_name_input: 'There was an error processing the onboarding request.',
1232-
},
1282+
// The handler should acknowledge successfully even if onboarding will fail
1283+
expect(mockAck).to.have.been.calledOnce;
1284+
expect(mockAck).to.have.been.calledWith();
1285+
1286+
// Wait a bit for the background promise to settle
1287+
await new Promise((resolve) => {
1288+
setTimeout(resolve, 100);
12331289
});
1290+
1291+
// The error should be logged by the background error handler
1292+
expect(lambdaCtx.log.error).to.have.been.calledWith(
1293+
sinon.match(/Error in background onboarding for site/),
1294+
sinon.match.instanceOf(Error),
1295+
);
12341296
});
12351297

12361298
it('should return validation error when IMS org ID is not provided', async () => {

0 commit comments

Comments
 (0)