Release v1.34.1 (#873)

* Ensure deleted failed steps are filtered when retrieving test
execution steps.
* Add automatic retries for 502s from M365 Excel
* Remove unused tiles queue interval environment variable

Author: kevinkim-ogp

ecs/env.json

@@ -221,10 +221,6 @@
     {
       "name": "ADMIN_JWT_SECRET_KEY",
       "valueFrom": "plumber-<ENVIRONMENT>-admin-jwt-secret-key"
-    },
-    {
-      "name": "TILES_INTERVAL_BETWEEN_FIND_SINGLE_ROW_MS",
-      "valueFrom": "plumber-<ENVIRONMENT>-tiles-interval-between-find-single-row-ms"
     }
   ]
 }

package-lock.json

@@ -105,5 +105,5 @@
     "tsconfig-paths": "^4.2.0",
     "type-fest": "4.10.3"
   },
-  "version": "1.34.0"
+  "version": "1.34.1"
 }

packages/backend/package.json

@@ -97,23 +97,26 @@ describe('M365 request error handlers', () => {
     vi.restoreAllMocks()
   })
 
-  it('logs a warning and throws a RetriableError with default step delay on 503, if response does not have retry-after', async () => {
-    mockAxiosAdapterToThrowOnce(503)
-    await http
-      .get('/test-url')
-      .then(() => {
-        expect.unreachable()
-      })
-      .catch((error): void => {
-        expect(error).toBeInstanceOf(RetriableError)
-        expect(error.delayType).toEqual('step')
-        expect(error.delayInMs).toEqual(DEFAULT_DELAY_MS)
-      })
-    expect(mocks.logWarning).toHaveBeenCalledWith(
-      expect.stringContaining('HTTP 503'),
-      expect.objectContaining({ event: 'm365-http-503' }),
-    )
-  })
+  it.each([500, 502, 503])(
+    'logs a warning and throws a RetriableError with default step delay on %s',
+    async (status) => {
+      mockAxiosAdapterToThrowOnce(status)
+      await http
+        .get('/test-url')
+        .then(() => {
+          expect.unreachable()
+        })
+        .catch((error): void => {
+          expect(error).toBeInstanceOf(RetriableError)
+          expect(error.delayType).toEqual('step')
+          expect(error.delayInMs).toEqual(DEFAULT_DELAY_MS)
+        })
+      expect(mocks.logWarning).toHaveBeenCalledWith(
+        expect.stringContaining(`HTTP ${status}`),
+        expect.objectContaining({ event: `m365-http-${status}` }),
+      )
+    },
+  )
 
   it('logs a warning and throws a RetriableError with step delay set to retry-after, on receiving 503', async () => {
     mockAxiosAdapterToThrowOnce(503, { 'retry-after': 123 })

packages/backend/src/apps/m365-excel/__tests__/common/interceptors.error-handlers.test.ts

@@ -59,7 +59,7 @@ const handle429: ThrowingHandler = ($, error) => {
 //
 // Retry failures due to flakey M365 servers
 //
-const handle500and503: ThrowingHandler = function ($, error) {
+const handle500and502and503: ThrowingHandler = function ($, error) {
   // Log to monitor spikes, just in case
   const status = error.response.status
   logger.warn(`Received HTTP ${status} from MS Graph`, {
@@ -115,8 +115,9 @@ const errorHandler: IApp['requestErrorHandler'] = async function ($, error) {
     case 429: // Rate limited
       return handle429($, error)
     case 500:
+    case 502: // Bad gateway
     case 503: // Transient error
-      return handle500and503($, error)
+      return handle500and502and503($, error)
     case 509: // Bandwidth limit reached
       return handle509($, error)
   }

packages/backend/src/apps/m365-excel/common/interceptors/request-error-handler.ts

@@ -29,7 +29,7 @@ export async function getTestExecutionSteps(
     /**
      * NOTE: filters out test execution steps for steps that have been deleted
      */
-    testExecutionSteps
+    const filteredTestExecutionSteps = testExecutionSteps
       .filter((e) => e.step)
       .sort((a, b) => a.step.position - b.step.position)
 
@@ -38,22 +38,25 @@ export async function getTestExecutionSteps(
      * If more than 1 exists, we return the latest one sorted by createdAt
      */
     const stepIds = new Set<string>()
-    const dedupedTestExecutionSteps = testExecutionSteps.reduce((acc, curr) => {
-      if (stepIds.has(curr.stepId)) {
-        const otherExecutionStep = acc[acc.length - 1]
-        // possible bug in single step testing !! this should not happen
-        console.warn(
-          `Bug: More than 1 execution step found for step ${curr.stepId}`,
-        )
-        if (curr.createdAt > otherExecutionStep.createdAt) {
-          acc[acc.length - 1] = curr
+    const dedupedTestExecutionSteps = filteredTestExecutionSteps.reduce(
+      (acc, curr) => {
+        if (stepIds.has(curr.stepId)) {
+          const otherExecutionStep = acc[acc.length - 1]
+          // possible bug in single step testing !! this should not happen
+          console.warn(
+            `Bug: More than 1 execution step found for step ${curr.stepId}`,
+          )
+          if (curr.createdAt > otherExecutionStep.createdAt) {
+            acc[acc.length - 1] = curr
+          }
+        } else {
+          stepIds.add(curr.stepId)
+          acc.push(curr)
         }
-      } else {
-        stepIds.add(curr.stepId)
-        acc.push(curr)
-      }
-      return acc
-    }, [] as ExecutionStep[])
+        return acc
+      },
+      [] as ExecutionStep[],
+    )
 
     return dedupedTestExecutionSteps
   }

packages/backend/src/helpers/get-test-execution-steps.ts

@@ -1,6 +1,6 @@
 {
   "name": "frontend",
-  "version": "1.34.0",
+  "version": "1.34.1",
   "scripts": {
     "dev": "wait-on tcp:3000 && vite --host --force",
     "build": "tsc && vite build --mode=${VITE_MODE:-prod}",

packages/frontend/package.json

@@ -2,5 +2,5 @@
   "name": "@plumber/types",
   "description": "Shared types for plumber",
   "types": "./index.d.ts",
-  "version": "1.34.0"
+  "version": "1.34.1"
 }