fix(webhook): do not retry after a successful trigger delivery

.changeset/webhook-trigger-retry-after-success.md

@@ -0,0 +1,7 @@
+---
+"@slack/webhook": patch
+---
+
+fix: stop `WebhookTrigger.send()` from retrying after a successful delivery
+
+A 2xx response with an empty or non-JSON body no longer throws. It now resolves to `{ ok: true }`. Before this fix that body threw a parse error, which was retried as a request error and caused duplicate Workflow Builder runs. A valid `{ ok: false, error }` body is still surfaced to the caller.

packages/webhook/src/WebhookTrigger.test.ts

@@ -77,6 +77,33 @@ describe('WebhookTrigger', () => {
         assert.strictEqual(result.ok, true);
         scope.done();
       });
+
+      it('should resolve to { ok: true } on an empty 2xx body', async () => {
+        const scope = nock('https://hooks.slack.com')
+          .post(/triggers/)
+          .reply(200);
+        const result = await trigger.send({ key: 'value' });
+        assert.deepStrictEqual(result, { ok: true });
+        scope.done();
+      });
+
+      it('should resolve to { ok: true } on a non-JSON 2xx body', async () => {
+        const scope = nock('https://hooks.slack.com')
+          .post(/triggers/)
+          .reply(200, 'ok');
+        const result = await trigger.send({ key: 'value' });
+        assert.deepStrictEqual(result, { ok: true });
+        scope.done();
+      });
+
+      it('should surface a valid { ok: false } JSON body', async () => {
+        const scope = nock('https://hooks.slack.com')
+          .post(/triggers/)
+          .reply(200, { ok: false, error: 'trigger_error' });
+        const result = await trigger.send({ key: 'value' });
+        assert.deepStrictEqual(result, { ok: false, error: 'trigger_error' });
+        scope.done();
+      });
     });
 
     describe('on failure', () => {
@@ -211,6 +238,16 @@ describe('WebhookTrigger', () => {
         scope.done();
       });
 
+      it('does not retry an empty 2xx body even when a retry policy is set', async () => {
+        const scope = nock('https://hooks.slack.com')
+          .post(/triggers/)
+          .reply(200);
+        const trigger = new WebhookTrigger(url, { retryConfig: rapidRetryPolicy });
+        const result = await trigger.send({ key: 'value' });
+        assert.deepStrictEqual(result, { ok: true });
+        scope.done();
+      });
+
       it('gives up with the HTTP error after exhausting retries', async () => {
         const scope = nock('https://hooks.slack.com')
           .post(/triggers/)

packages/webhook/src/WebhookTrigger.ts

@@ -1,7 +1,7 @@
 import pRetry, { AbortError } from 'p-retry';
 
 import { SlackWebhookError, WebhookTriggerHTTPError, WebhookTriggerRequestError } from './errors';
-import type { FetchFunction } from './IncomingWebhook';
+import type { FetchFunction, FetchResponse } from './IncomingWebhook';
 import { getUserAgent } from './instrument';
 import type { RetryOptions } from './retry-policies';
 
@@ -81,7 +81,7 @@ export class WebhookTrigger {
           throw response.status >= 500 ? httpError : new AbortError(httpError);
         }
 
-        return (await response.json()) as WebhookTriggerResult;
+        return await this.buildResult(response);
       } catch (error) {
         // Non-retryable signals (AbortError) and already-wrapped errors pass through untouched.
         if (error instanceof AbortError || error instanceof SlackWebhookError) {
@@ -92,6 +92,18 @@ export class WebhookTrigger {
       }
     }, this.retryConfig);
   }
+
+  private async buildResult(response: FetchResponse): Promise<WebhookTriggerResult> {
+    const text = await response.text();
+    try {
+      if (!text) {
+        return { ok: true };
+      }
+      return JSON.parse(text) as WebhookTriggerResult;
+    } catch {
+      return { ok: true };
+    }
+  }
 }
 
 /*