Skip to content

Commit 7c08bec

Browse files
committed
chore(webhooks): tighten poll comments and backoff tests
Poll outcome docs describe what a skipped poll actually does, source failures keep logging the full error object as the pollers did before, the backoff table pins the clock past the poll start so it proves the window is anchored there, and the RSS rate-limit test drives a Retry-After header.
1 parent 7cdf6b1 commit 7c08bec

5 files changed

Lines changed: 16 additions & 20 deletions

File tree

‎apps/sim/lib/webhooks/polling/rss.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,10 @@ describe('RSS polling against refusals and rate limits', () => {
160160

161161
it("records a rate-limited fetch as one failure carrying the source's requested wait", async () => {
162162
mockFetch.mockResolvedValue(
163-
new Response('{"ok":false,"description":"Too Many Requests: FLOOD_WAIT_12"}', {
163+
new Response('Too Many Requests', {
164164
status: 429,
165165
statusText: 'Too Many Requests',
166+
headers: { 'Retry-After': '12' },
166167
})
167168
)
168169

‎apps/sim/lib/webhooks/polling/rss.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,7 @@ async function fetchNewRssItems(
285285
return false
286286
}
287287

288-
/**
289-
* A cached feed can reveal an item after its publication time. Only the fixed
290-
* subscription boundary excludes history; the last poll time is not a delivery cursor.
291-
*/
288+
// Cached feeds reveal items late; only the subscription boundary excludes history, never the last poll.
292289
if (item.isoDate) {
293290
const itemDate = new Date(item.isoDate)
294291
if (itemDate <= subscriptionStartedAt) {

‎apps/sim/lib/webhooks/polling/types.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ import type { webhook, workflow } from '@sim/db/schema'
22
import type { Logger } from '@sim/logger'
33
import { toRecord } from '@sim/utils/object'
44

5-
/** Outcome of one webhook's poll; `skipped` polls fetched nothing and changed no state. */
5+
/** Outcome of one webhook's poll; a `skipped` poll counts as neither a success nor a failure. */
66
export type PollOutcome = 'success' | 'failure' | 'skipped'
77

88
/** Summary returned after polling all webhooks for a provider. */
99
export interface PollSummary {
1010
total: number
1111
successful: number
1212
failed: number
13-
/** Not polled this tick: backing off after failures, or the payer is over its usage limit. */
13+
/** Not polled this tick (source backoff or a recorded admission refusal), or stopped mid-poll on one. */
1414
skipped: number
1515
}
1616

@@ -55,7 +55,7 @@ export interface PollingProviderHandler {
5555
/**
5656
* Process a single webhook entry.
5757
* Return 'success' (even if 0 new items), 'failure', or 'skipped' when the
58-
* poll stopped without consuming anything (an admission rejection mid-poll).
58+
* poll stopped on an admission refusal and left the rest for a later poll.
5959
*/
6060
pollWebhook(ctx: PollWebhookContext): Promise<PollOutcome>
6161
}

‎apps/sim/lib/webhooks/polling/utils.test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
22
import { authOAuthUtilsMock } from '@sim/testing/mocks/auth-oauth-utils.mock'
3-
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
3+
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
44

55
vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)
66
vi.mock('@/triggers/constants', () => ({ MAX_CONSECUTIVE_FAILURES: 5 }))
@@ -58,6 +58,11 @@ describe('poll source backoff', () => {
5858

5959
beforeEach(() => {
6060
resetDbChainMock()
61+
vi.useFakeTimers({ now: pollStartedAt + minutes(5) })
62+
})
63+
64+
afterEach(() => {
65+
vi.useRealTimers()
6166
})
6267

6368
/** Records one source failure on a webhook whose config carries `previousFailures`, returning the merged config update. */
@@ -95,9 +100,9 @@ describe('poll source backoff', () => {
95100
}
96101
)
97102

98-
it('keeps a webhook backed off until its window ends', async () => {
99-
const stored = await failOnce(4)
100-
const until = Date.parse(String(stored.pollBackoffUntil))
103+
it('keeps a webhook backed off until its window ends', () => {
104+
const until = pollStartedAt + minutes(16)
105+
const stored = { pollBackoffUntil: new Date(until).toISOString() }
101106

102107
expect(isPollBackedOff(stored, until - minutes(1))).toBe(true)
103108
expect(isPollBackedOff(stored, until)).toBe(false)
@@ -108,11 +113,6 @@ describe('poll source backoff', () => {
108113
expect(Date.parse(String(stored.pollBackoffUntil))).toBe(pollStartedAt + minutes(10))
109114
})
110115

111-
it('lets the next tick poll after one failure even when the failing poll ran long', async () => {
112-
const stored = await failOnce(0)
113-
expect(isPollBackedOff(stored, pollStartedAt + minutes(1.2))).toBe(false)
114-
})
115-
116116
it('ignores a missing or malformed window', () => {
117117
for (const config of [{}, { pollBackoffUntil: 'not-a-date' }, { pollBackoffUntil: 42 }, null]) {
118118
expect(isPollBackedOff(config, pollStartedAt)).toBe(false)

‎apps/sim/lib/webhooks/polling/utils.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { db } from '@sim/db'
22
import { account, webhook, workflow, workflowDeploymentVersion } from '@sim/db/schema'
33
import type { Logger } from '@sim/logger'
44
import { toNumberOrNull } from '@sim/utils/coerce'
5-
import { getErrorMessage } from '@sim/utils/errors'
65
import { toRecord } from '@sim/utils/object'
76
import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry'
87
import { and, eq, isNull, ne, or, sql } from 'drizzle-orm'
@@ -56,7 +55,6 @@ export class PollFetchError extends Error {
5655
}
5756
}
5857

59-
/** Reads the wait a rate-limited response asked for, in milliseconds. */
6058
export function readPollRetryAfterMs(retryAfterHeader: string | null, body: string): number | null {
6159
const fromHeader = parseRetryAfter(retryAfterHeader, POLL_RETRY_AFTER_MAX_MS)
6260
if (fromHeader !== null) return fromHeader
@@ -88,7 +86,7 @@ export async function recordPollSourceFailure(
8886
...(retryAfterMs !== null ? { retryAfterMs } : {}),
8987
})
9088
} else {
91-
logger.error(message, { error: getErrorMessage(error, 'Unknown error') })
89+
logger.error(message, error)
9290
}
9391

9492
const failures =

0 commit comments

Comments
 (0)