Skip to content

Commit 82273c5

Browse files
Polling service: rate limiter fixes (#231)
* Polling service: rate limiter fixes * Refactor AdminAPI rate limiting from setInterval to promise chaining - Replace setInterval-based processing with promise chain approach - Add async versions of batch processing methods (doBatchPreviewAsync, doBatchPublishAsync, doBatchUnpublishAsync) - Improve error handling and resilience in processing chain - Fix infinite loop issues when no work is available - Update tests to work with new promise-based approach - Add proper cleanup and state management - Prevent service from getting stuck in certain conditions This makes the service more robust and prevents issues where the interval might continue triggering until the action is terminated (global timeout of 1h). * Fix ESLint errors and
1 parent fee1063 commit 82273c5

4 files changed

Lines changed: 300 additions & 39 deletions

File tree

actions/lib/aem.js

Lines changed: 258 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,13 @@ class AdminAPI {
4141
this.publishBatchSize = publishBatchSize;
4242
this.authToken = authToken;
4343
this.context = context;
44-
this.onQueuesProcessed = null;
45-
this.stopProcessing$ = null;
44+
this.isProcessing = false;
45+
this.processingPromise = null;
46+
this.shouldStop = false;
4647
this.lastStatusLog = 0;
4748
this.previewDurations = [];
4849
this.queue = [];
50+
this.processingChain = null;
4951
}
5052

5153
previewAndPublish(records, locale, batchNumber) {
@@ -61,38 +63,109 @@ class AdminAPI {
6163
}
6264

6365
async startProcessing() {
64-
if (this.stopProcessing$) {
65-
// only restart processing after awaiting stopProcessing
66-
await this.stopProcessing$;
66+
if (this.isProcessing) {
67+
return this.processingPromise;
6768
}
68-
if (!this.interval) {
69-
this.interval = setInterval(() => this.processQueues(), 1000);
69+
70+
this.isProcessing = true;
71+
this.shouldStop = false;
72+
73+
this.processingPromise = this.processQueuesWithPromiseChain();
74+
return this.processingPromise;
75+
}
76+
77+
async stopProcessing() {
78+
this.shouldStop = true;
79+
80+
if (this.processingPromise) {
81+
await this.processingPromise;
7082
}
83+
84+
this.isProcessing = false;
85+
this.processingPromise = null;
7186
}
7287

73-
stopProcessing() {
74-
if (!this.interval) {
88+
async processQueuesWithPromiseChain() {
89+
const { logger } = this.context;
90+
91+
while (!this.shouldStop) {
92+
if (!this.hasWorkToDo()) {
93+
// No work to do, exit the loop
94+
break;
95+
}
96+
97+
try {
98+
await this.processNextBatch();
99+
100+
// Small delay to prevent overwhelming the system
101+
await this.delay(100);
102+
103+
// Log status periodically
104+
if (this.lastStatusLog < new Date() - 1000) {
105+
this.logQueueStatus();
106+
this.lastStatusLog = new Date();
107+
}
108+
} catch (error) {
109+
logger.error('Error in processing chain:', error);
110+
// Continue processing even if one batch fails
111+
await this.delay(1000); // Longer delay on error
112+
}
113+
}
114+
115+
logger.info('Processing chain completed');
116+
117+
// Reset processing state when chain completes
118+
this.isProcessing = false;
119+
this.processingPromise = null;
120+
}
121+
122+
hasWorkToDo() {
123+
return this.previewQueue.length > 0 ||
124+
this.publishQueue.length > 0 ||
125+
this.unpublishQueue.length > 0 ||
126+
this.unpublishPreviewQueue.length > 0 ||
127+
this.inflight.length > 0;
128+
}
129+
130+
async processNextBatch() {
131+
// Process queues in priority order
132+
if (this.publishQueue.length > 0) {
133+
const batch = this.publishQueue.shift();
134+
await this.doBatchPublishAsync(batch);
75135
return;
76136
}
77-
// stopProcessing only once by keeping a single promise resolving after all queues are processed
78-
if (!this.stopProcessing$) {
79-
this.stopProcessing$ = new Promise((resolve) => {
80-
this.onQueuesProcessed = () => {
81-
if (this.previewQueue.length + this.publishQueue.length + this.unpublishQueue.length + this.unpublishPreviewQueue.length + this.inflight.length > 0) {
82-
// still running
83-
return;
84-
}
85137

86-
// reset callback
87-
clearInterval(this.interval);
88-
this.onQueuesProcessed = null;
89-
this.stopProcessing$ = null;
90-
this.interval = null;
91-
resolve();
92-
};
93-
});
138+
if (this.previewQueue.length > 0) {
139+
const batch = this.previewQueue.shift();
140+
await this.doBatchPreviewAsync(batch);
141+
return;
142+
}
143+
144+
if (this.unpublishQueue.length > 0) {
145+
const batch = this.unpublishQueue.shift();
146+
await this.doBatchUnpublishAsync(batch, 'live');
147+
return;
148+
}
149+
150+
if (this.unpublishPreviewQueue.length > 0) {
151+
const batch = this.unpublishPreviewQueue.shift();
152+
await this.doBatchUnpublishAsync(batch, 'preview');
153+
return;
94154
}
95-
return this.stopProcessing$;
155+
}
156+
157+
logQueueStatus() {
158+
const { logger } = this.context;
159+
logger.info(`Queues: preview=${this.previewQueue.length},`
160+
+ ` publish=${this.publishQueue.length},`
161+
+ ` unpublish live=${this.unpublishQueue.length},`
162+
+ ` unpublish preview=${this.unpublishPreviewQueue.length},`
163+
+ ` inflight=${this.inflight.length},`
164+
+ ` in queue=${this.queue.length}`);
165+
}
166+
167+
delay(ms) {
168+
return new Promise(resolve => setTimeout(resolve, ms));
96169
}
97170

98171
trackInFlight(name, callback) {
@@ -270,6 +343,56 @@ class AdminAPI {
270343
});
271344
}
272345

346+
async doBatchPreviewAsync(batch) {
347+
const { logger } = this.context;
348+
const { records, locale, batchNumber } = batch;
349+
const paths = records.map(record => record.path);
350+
351+
if (paths.length === 0) {
352+
logger.info(`Skipping preview for batch id=${batchNumber} for locale=${locale}: no paths to process.`);
353+
batch.resolve({ records, locale, batchNumber });
354+
return;
355+
}
356+
357+
const body = {
358+
forceUpdate: true,
359+
paths,
360+
delete: false
361+
};
362+
const start = new Date();
363+
364+
try {
365+
// Try to preview the batch using bulk preview API
366+
const response = await this.runWithRetry(
367+
async () => {
368+
return await this.execAdminRequest('POST', 'preview', '/*', body);
369+
},
370+
`preview batch number ${batchNumber} for locale ${locale}`
371+
);
372+
373+
if (response?.job) {
374+
logger.info(`Previewed batch number ${batchNumber} for locale ${locale}`);
375+
const successPaths = await this.checkJobStatus(response.job);
376+
batch.records.forEach(record => {
377+
if (successPaths.includes(record.path)) {
378+
record.previewedAt = new Date();
379+
}
380+
});
381+
382+
this.publishQueue.push(batch);
383+
} else {
384+
logger.error(`Error previewing batch number ${batchNumber} for locale ${locale}`);
385+
batch.resolve({records, locale, batchNumber});
386+
}
387+
} catch (error) {
388+
logger.error(`Error in preview batch ${batchNumber}:`, error);
389+
batch.resolve({records, locale, batchNumber});
390+
}
391+
392+
// Complete the batch preview
393+
this.previewDurations.push(new Date() - start);
394+
}
395+
273396
doBatchPublish(batch) {
274397
this.trackInFlight('publish', async (complete) => {
275398
const { logger } = this.context;
@@ -315,6 +438,51 @@ class AdminAPI {
315438
});
316439
}
317440

441+
async doBatchPublishAsync(batch) {
442+
const { logger } = this.context;
443+
const { records, locale, batchNumber } = batch;
444+
const paths = records.filter(record => record.previewedAt).map(record => record.path);
445+
446+
if (paths.length === 0) {
447+
logger.info(`Skipping publish in batch id=${batchNumber} for locale=${locale}: no paths to process.`);
448+
batch.resolve({ records, locale, batchNumber });
449+
return;
450+
}
451+
452+
const body = {
453+
forceUpdate: true,
454+
paths,
455+
delete: false
456+
};
457+
458+
try {
459+
// Try to publish the batch using bulk publish API
460+
const response = await this.runWithRetry(
461+
async () => {
462+
return await this.execAdminRequest('POST', 'live', '/*', body);
463+
},
464+
`publish batch number ${batchNumber} for locale ${locale}`
465+
);
466+
467+
if (response?.job) {
468+
logger.info(`Published batch number ${batchNumber} for locale ${locale}`);
469+
const successPaths = await this.checkJobStatus(response.job);
470+
batch.records.forEach(record => {
471+
if (successPaths.includes(record.path)) {
472+
record.publishedAt = new Date();
473+
}
474+
});
475+
} else {
476+
logger.error(`Error publishing batch number ${batchNumber} for locale ${locale}`);
477+
}
478+
} catch (error) {
479+
logger.error(`Error in publish batch ${batchNumber}:`, error);
480+
}
481+
482+
// Resolve the original promises
483+
batch.resolve({records, locale, batchNumber});
484+
}
485+
318486
doBatchUnpublish(batch, route) {
319487
this.trackInFlight('unpublish', async (complete) => {
320488
const { logger } = this.context;
@@ -378,6 +546,70 @@ class AdminAPI {
378546
});
379547
}
380548

549+
async doBatchUnpublishAsync(batch, route) {
550+
const { logger } = this.context;
551+
const { records, locale, batchNumber } = batch;
552+
553+
const paths = route === 'live'
554+
? records.map(record => record.path)
555+
: records.filter(record => record.liveUnpublishedAt).map(record => record.path);
556+
557+
if (paths.length === 0) {
558+
logger.info(`Skipping unpublish for route=${route} in batch id=${batchNumber} for locale=${locale}: no paths to process.`);
559+
batch.resolve({ records, locale, batchNumber });
560+
return;
561+
}
562+
563+
const body = {
564+
forceUpdate: true,
565+
paths,
566+
delete: true,
567+
};
568+
569+
try {
570+
// Try to unpublish live the batch using bulk publish API
571+
const response = await this.runWithRetry(
572+
async () => {
573+
return await this.execAdminRequest('POST', route, '/*', body);
574+
},
575+
`unpublish ${route} batch number ${batchNumber} for locale ${locale}`
576+
);
577+
578+
if (response?.job) {
579+
logger.info(`Unpublished ${route} batch number ${batchNumber} for locale ${locale}`);
580+
const successPaths = await this.checkJobStatus(response.job);
581+
batch.records.forEach(record => {
582+
if (successPaths.includes(record.path)) {
583+
if (route === 'live') {
584+
record.liveUnpublishedAt = new Date();
585+
} else {
586+
record.previewUnpublishedAt = new Date();
587+
}
588+
}
589+
});
590+
591+
if (route === 'live') {
592+
this.unpublishPreviewQueue.push(batch);
593+
}
594+
} else {
595+
logger.error(`Error unpublishing ${route} batch number ${batchNumber} for locale ${locale}`);
596+
if (route === 'live') {
597+
batch.resolve({records, locale, batchNumber});
598+
}
599+
}
600+
} catch (error) {
601+
logger.error(`Error in unpublish batch ${batchNumber} for route ${route}:`, error);
602+
if (route === 'live') {
603+
batch.resolve({records, locale, batchNumber});
604+
}
605+
}
606+
607+
// Resolve the original promises
608+
if (route === 'preview') {
609+
batch.resolve({records, locale, batchNumber});
610+
}
611+
}
612+
381613
processQueues() {
382614
if (this.lastStatusLog < new Date() - 1000) {
383615
const { logger } = this.context;

actions/lib/runtimeConfig.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
/* Centralized runtime config resolver for AppBuilder actions (CommonJS) */
2-
/* eslint-disable no-console */
32
const { isValidUrl } = require('../utils');
43

54
const DEFAULTS = {

0 commit comments

Comments
 (0)