Tests for applyUpdateStageForChunk (#143)

* Tests for applyUpdateStageForChunk

* Update src/stageCache.js

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: DianaIonita

package-lock.json

@@ -30,10 +30,12 @@
   },
   "devDependencies": {
     "chai": "^4.1.2",
+    "chai-as-promised": "^8.0.1",
     "chance": "^1.0.16",
     "lodash.split": "^4.4.0",
     "mocha": "^10.0.0",
     "mocha-junit-reporter": "^2.0.2",
-    "proxyquire": "^2.1.0"
+    "proxyquire": "^2.1.0",
+    "sinon": "^20.0.0"
   }
 }

package.json

@@ -2,7 +2,7 @@
 
 const ApiGatewayCachingSettings = require('./ApiGatewayCachingSettings');
 const cacheKeyParameters = require('./cacheKeyParameters');
-const updateStageCacheSettings = require('./stageCache');
+const { updateStageCacheSettings } = require('./stageCache');
 const { restApiExists, outputRestApiIdTo } = require('./restApiId');
 
 class ApiGatewayCachingPlugin {

src/apiGatewayCachingPlugin.js

@@ -1,6 +1,7 @@
 const isEmpty = require('lodash.isempty');
 const { retrieveRestApiId } = require('./restApiId');
 const MAX_PATCH_OPERATIONS_PER_STAGE_UPDATE = 80;
+const BASE_RETRY_DELAY_MS = 500;
 
 String.prototype.replaceAll = function (search, replacement) {
   let target = this;
@@ -194,38 +195,48 @@ const updateStageFor = async (serverless, params, stage, region) => {
     paramsInChunks.push(params);
   }
 
-  for (let index in paramsInChunks) {
-    serverless.cli.log(`[serverless-api-gateway-caching] Updating API Gateway cache settings (${parseInt(index) + 1} of ${paramsInChunks.length}).`);
-    await applyUpdateStageForChunk(paramsInChunks[index], serverless, stage, region);
+  for (const [index, chunk] of paramsInChunks.entries()) {
+    serverless.cli.log(`[serverless-api-gateway-caching] Updating API Gateway cache settings (${index + 1} of ${paramsInChunks.length}).`);
+    await applyUpdateStageForChunk(chunk, serverless, stage, region);
   }
 
   serverless.cli.log(`[serverless-api-gateway-caching] Done updating API Gateway cache settings.`);
 }
 
 const applyUpdateStageForChunk = async (chunk, serverless, stage, region) => {
   const maxRetries = 10;
-  const baseDelay = 500;
+  const baseDelay = BASE_RETRY_DELAY_MS;
   let attempt = 0;
 
-  while (attempt <= maxRetries) {
+  while (attempt < maxRetries) {
     try {
       serverless.cli.log(`[serverless-api-gateway-caching] Updating API Gateway cache settings. Attempt ${attempt + 1}.`);
       await serverless.providers.aws.request('APIGateway', 'updateStage', chunk, stage, region);
-      break;
+      return;
     } catch (error) {
+      // Check for specific error code first, fallback to message check
       if (
-          attempt < maxRetries &&
-          error.message.includes('A previous change is still in progress')
+          (error.code === 'ConflictException' || error.message.includes('A previous change is still in progress'))
       ) {
         attempt++;
-        const delay = baseDelay * 2 ** (attempt - 1);
+        if (attempt >= maxRetries) {
+           serverless.cli.log(`[serverless-api-gateway-caching] Maximum retries (${maxRetries}) reached. Failed to update API Gateway cache settings.`);
+           // Log the full error for better debugging before throwing
+           console.error('[serverless-api-gateway-caching] Final error details:', error);
+           throw new Error(`Failed to update API Gateway cache settings after ${maxRetries} retries: ${error.message}`);
+        }
+        const delay = baseDelay * 2 ** attempt;
         serverless.cli.log(`[serverless-api-gateway-caching] Retrying (${attempt}/${maxRetries}) after ${delay}ms due to error: ${error.message}`);
         await new Promise((resolve) => setTimeout(resolve, delay));
       } else {
-        throw new Error(`Failed to update API Gateway cache settings after ${attempt} retries: ${error.message}`);
+        console.error('[serverless-api-gateway-caching] Non-retryable error during update:', error);
+        // Throw immediately for non-retryable errors or if string/code doesn't match
+        throw new Error(`Failed to update API Gateway cache settings: ${error.message}`);
       }
     }
   }
+  // This part should ideally not be reached if the loop condition is < maxRetries and success returns early.
+  // Removing the final safeguard as it is redundant.
 }
 
 const updateStageCacheSettings = async (settings, serverless) => {
@@ -266,4 +277,7 @@ const updateStageCacheSettings = async (settings, serverless) => {
   await updateStageFor(serverless, params, settings.stage, settings.region);
 }
 
-module.exports = updateStageCacheSettings;
+module.exports = {
+  updateStageCacheSettings,
+  applyUpdateStageForChunk
+}

src/stageCache.js

@@ -90,14 +90,16 @@ describe('Creating plugin', () => {
         const serverless = { cli: { log: (message) => { logCalledWith = message } } };
         const restApiIdStub = {
           restApiExists: () => scenario.thereIsARestApi,
-          outputRestApiIdTo: () => outputRestApiIdCalled = true
+          outputRestApiIdTo: () => {}
+        };
+        const stageCacheStub = {
+          updateStageCacheSettings: () => updateStageCacheSettingsCalled = true
         };
-        const stageCacheStub = () => updateStageCacheSettingsCalled = true;
         const ApiGatewayCachingPlugin = proxyquire('../src/apiGatewayCachingPlugin', { './restApiId': restApiIdStub, './stageCache': stageCacheStub });
         const plugin = new ApiGatewayCachingPlugin(serverless, {});
 
-        before(() => {
-          plugin.updateStage();
+        before(async () => {
+          await plugin.updateStage();
         });
 
         it('should log a message', () => {

test/creating-plugin.js

@@ -1,4 +1,4 @@
-const updateStageCacheSettings = require('../../src/stageCache');
+const { updateStageCacheSettings } = require('../../src/stageCache');
 
 const updating_stage_cache_settings = async (settings, serverless) => {
     return await updateStageCacheSettings(settings, serverless);

test/steps/when.js

@@ -1,7 +1,7 @@
 const given = require('../test/steps/given');
 const when = require('../test/steps/when');
 const ApiGatewayCachingSettings = require('../src/ApiGatewayCachingSettings');
-const updateStageCacheSettings = require('../src/stageCache');
+const { updateStageCacheSettings } = require('../src/stageCache');
 const expect = require('chai').expect;
 
 describe('Updating stage cache settings for additional endpoints defined as CloudFormation', () => {