fix(termination-watcher): add retry for runner-busy 422 and fix undefined log message

- Add exponential backoff retry (5 attempts, 1s/2s/4s/8s/16s) when
  GitHub returns 422 "currently running a job" during runner deletion
- Fix "Received spot notification for undefined" log message when
  metrics are disabled by omitting the metric name from the log

Author: jensenbox

lambdas/functions/termination-watcher/src/deregister.test.ts

@@ -282,6 +282,77 @@ describe('deregisterRunner', () => {
     await deregisterRunner(instance, baseConfig);
     expect(mockCreateAppAuth).not.toHaveBeenCalled();
   });
+
+  it('should retry when runner is currently running a job (422)', async () => {
+    vi.useFakeTimers();
+    mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
+
+    async function* fakeIterator() {
+      yield { data: [{ id: 42, name: `runner-i-12345678901234567` }] };
+    }
+    mockPaginate.iterator.mockReturnValue(fakeIterator());
+
+    const busyError = Object.assign(new Error('Bad request - Runner runner-i-12345678901234567 is currently running a job and cannot be deleted.'), { status: 422 });
+    mockActions.deleteSelfHostedRunnerFromOrg
+      .mockRejectedValueOnce(busyError)
+      .mockRejectedValueOnce(busyError)
+      .mockResolvedValueOnce({});
+
+    const promise = deregisterRunner(orgInstance, baseConfig);
+
+    // Advance through retry delays (1s, 2s)
+    await vi.advanceTimersByTimeAsync(1000);
+    await vi.advanceTimersByTimeAsync(2000);
+
+    await promise;
+
+    expect(mockActions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalledTimes(3);
+    vi.useRealTimers();
+  });
+
+  it('should not retry on non-422 errors', async () => {
+    mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
+
+    async function* fakeIterator() {
+      yield { data: [{ id: 42, name: `runner-i-12345678901234567` }] };
+    }
+    mockPaginate.iterator.mockReturnValue(fakeIterator());
+
+    mockActions.deleteSelfHostedRunnerFromOrg.mockRejectedValueOnce(
+      Object.assign(new Error('Internal Server Error'), { status: 500 }),
+    );
+
+    // Should not throw — outer catch handles it
+    await deregisterRunner(orgInstance, baseConfig);
+
+    expect(mockActions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalledTimes(1);
+  });
+
+  it('should give up after max retries for runner busy error', async () => {
+    vi.useFakeTimers();
+    mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
+
+    async function* fakeIterator() {
+      yield { data: [{ id: 42, name: `runner-i-12345678901234567` }] };
+    }
+    mockPaginate.iterator.mockReturnValue(fakeIterator());
+
+    const busyError = Object.assign(new Error('Bad request - Runner is currently running a job and cannot be deleted.'), { status: 422 });
+    mockActions.deleteSelfHostedRunnerFromOrg.mockRejectedValue(busyError);
+
+    const promise = deregisterRunner(orgInstance, baseConfig);
+
+    // Advance through all retry delays (1s, 2s, 4s, 8s)
+    for (const delay of [1000, 2000, 4000, 8000]) {
+      await vi.advanceTimersByTimeAsync(delay);
+    }
+
+    await promise;
+
+    // 5 attempts total (initial + 4 retries)
+    expect(mockActions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalledTimes(5);
+    vi.useRealTimers();
+  });
 });
 
 describe('createThrottleOptions', () => {

lambdas/functions/termination-watcher/src/deregister.ts

@@ -155,6 +155,51 @@ async function deleteRunner(octokit: Octokit, owner: string, runnerId: number, r
   }
 }
 
+const MAX_DELETE_RETRIES = 5;
+const INITIAL_RETRY_DELAY_MS = 1000;
+
+async function deleteRunnerWithRetry(
+  octokit: Octokit,
+  owner: string,
+  runnerId: number,
+  runnerName: string,
+  runnerType: string,
+  instanceId: string,
+): Promise<void> {
+  for (let attempt = 1; attempt <= MAX_DELETE_RETRIES; attempt++) {
+    try {
+      await deleteRunner(octokit, owner, runnerId, runnerType);
+      logger.info('Successfully deregistered runner from GitHub', {
+        instanceId,
+        runnerId,
+        runnerName,
+        owner,
+      });
+      return;
+    } catch (error) {
+      const status = (error as { status?: number }).status;
+      const message = (error as Error).message ?? '';
+      const isRunnerBusy = status === 422 && message.includes('currently running a job');
+
+      if (isRunnerBusy && attempt < MAX_DELETE_RETRIES) {
+        const delay = INITIAL_RETRY_DELAY_MS * Math.pow(2, attempt - 1);
+        logger.warn('Runner is currently running a job, retrying after delay', {
+          instanceId,
+          runnerId,
+          runnerName,
+          owner,
+          attempt,
+          maxRetries: MAX_DELETE_RETRIES,
+          delayMs: delay,
+        });
+        await new Promise((resolve) => setTimeout(resolve, delay));
+      } else {
+        throw error;
+      }
+    }
+  }
+}
+
 export async function deregisterRunner(instance: Instance, config: Config): Promise<void> {
   if (!config.enableRunnerDeregistration) {
     logger.debug('Runner deregistration is disabled, skipping');
@@ -187,13 +232,7 @@ export async function deregisterRunner(instance: Instance, config: Config): Prom
       return;
     }
 
-    await deleteRunner(installationOctokit, owner, runner.id, runnerType);
-    logger.info('Successfully deregistered runner from GitHub', {
-      instanceId,
-      runnerId: runner.id,
-      runnerName: runner.name,
-      owner,
-    });
+    await deleteRunnerWithRetry(installationOctokit, owner, runner.id, runner.name, runnerType, instanceId);
   } catch (error) {
     logger.error('Failed to deregister runner from GitHub', {
       instanceId,

lambdas/functions/termination-watcher/src/metric-event.ts

@@ -13,7 +13,7 @@ export async function metricEvent(
   const instanceRunningTimeInSeconds = instance.LaunchTime
     ? (new Date(event.time).getTime() - new Date(instance.LaunchTime).getTime()) / 1000
     : undefined;
-  logger.info(`Received spot notification for ${metricName}`, {
+  logger.info(`Received spot notification${metricName ? ` for ${metricName}` : ''}`, {
     instanceId: instance.InstanceId,
     instanceType: instance.InstanceType ?? 'unknown',
     instanceName: instance.Tags?.find((tag) => tag.Key === 'Name')?.Value,