[FIX] Airtable webhook sources intermittently emit the raw Airtable payload #18237
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughIntroduces HMAC signature validation and cursor-based payload retrieval in common webhook logic, adds retry handling for record fetches, removes duplicate-event state logic, adds payload filtering hooks across sources, updates some prop definitions to use common props, and bumps versions and dependencies in package.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor AT as Airtable
participant WH as Webhook (common-webhook.run)
participant API as Airtable API
participant SRC as Source (per feature)
AT->>WH: HTTP POST webhook (bodyRaw, x-airtable-content-mac)
WH->>WH: Validate HMAC signature
alt signature invalid
WH-->>AT: 401 Unauthorized
else signature valid
WH->>WH: Load lastCursor, macSecret
WH->>API: listWebhookPayloads({ cursor:lastCursor, debug:true })
API-->>WH: { payloads[], cursor }
WH->>WH: filteredPayloads = payloads.filter(payloadFilter)
loop For each filtered payload
WH->>SRC: emitEvent(payload)
note over SRC: Emits normalized event or calls emitDefaultEvent({ originalPayload })
end
WH->>WH: _setLastCursor(cursor)
WH-->>AT: 200 OK
end
sequenceDiagram
autonumber
participant SRC as common-webhook-record.emitEvent
participant RET as withRetries(async-retry)
participant API as Airtable API
SRC->>RET: getRecord({ baseId, tableId, recordId })
alt transient errors
RET->>API: GET record (retry up to N)
API-->>RET: Error (e.g., not yet consistent)
RET-->>SRC: throw to catch
SRC->>SRC: fields = {}
else success
RET->>API: GET record
API-->>RET: { fields }
RET-->>SRC: fields
end
SRC-->>WH: Emit event with summary and fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
components/airtable_oauth/sources/new-records/new-records.mjs (1)
13-15: Harden payload gating: require non-empty changedTablesById.Empty objects pass the current truthy check and can still lead to downstream no-op/edge cases.
Apply:
- payloadFilter(payload) { - return !!payload.changedTablesById; - }, + payloadFilter(payload) { + const t = payload?.changedTablesById; + return t && typeof t === "object" && Object.keys(t).length > 0; + },components/airtable_oauth/sources/new-or-modified-field/new-or-modified-field.mjs (1)
14-16: Tighten payloadFilter to avoid passing empty objects.Same rationale as other sources.
Apply:
- payloadFilter(payload) { - return !!payload.changedTablesById; - }, + payloadFilter(payload) { + const t = payload?.changedTablesById; + return t && typeof t === "object" && Object.keys(t).length > 0; + },components/airtable_oauth/sources/new-field/new-field.mjs (1)
13-15: Make payloadFilter robust (non-empty object).Prevents letting through empty changedTablesById.
Apply:
- payloadFilter(payload) { - return !!payload.changedTablesById; - }, + payloadFilter(payload) { + const t = payload?.changedTablesById; + return t && typeof t === "object" && Object.keys(t).length > 0; + },components/airtable_oauth/sources/new-modified-or-deleted-records-instant/new-modified-or-deleted-records-instant.mjs (1)
44-46: Strengthen filter: require non-empty changedTablesById.Consistent with other sources and avoids edge cases.
Apply:
- payloadFilter(payload) { - return !!payload.changedTablesById; - }, + payloadFilter(payload) { + const t = payload?.changedTablesById; + return t && typeof t === "object" && Object.keys(t).length > 0; + },components/airtable_oauth/sources/new-or-modified-records/new-or-modified-records.mjs (1)
13-15: Tighten payload filter to avoid empty changedTablesById edge caseGuard against empty objects so emitEvent’s Object.entries(...) doesn’t throw on rare empty payloads.
- payloadFilter(payload) { - return !!payload.changedTablesById; - }, + payloadFilter(payload) { + const ct = payload?.changedTablesById; + return !!(ct && Object.keys(ct).length); + },components/airtable_oauth/sources/common/common-webhook-record.mjs (2)
62-71: Record fetch fallback is good; add minimal logging for observabilityKeeping fields = {} satisfies the “best-effort” requirement. Consider logging the final error once per event to aid debugging without crashing.
- } catch (e) { - fields = {}; - } + } catch (e) { + // best-effort: emit without fields + fields = {}; + console.debug?.("Airtable getRecord failed; emitting without fields", { + tableId, recordId, message: e?.message, + }); + }
24-33: Optional: defensively handle unexpected payload shapes inside emitEventEven with source-level payloadFilter, add a local guard to avoid destructuring empty objects.
async emitEvent(payload) { - const [ - tableId, - tableData, - ] = Object.entries(payload.changedTablesById)[0]; + const tableEntries = Object.entries(payload?.changedTablesById ?? {}); + if (!tableEntries.length) return this.emitDefaultEvent(payload); + const [tableId, tableData] = tableEntries[0]; @@ - let [ - operation, - recordObj, - ] = Object.entries(tableData)[0]; + const opEntries = Object.entries(tableData ?? {}); + if (!opEntries.length) return this.emitDefaultEvent(payload); + let [operation, recordObj] = opEntries[0];Also applies to: 53-61
components/airtable_oauth/sources/common/common-webhook.mjs (4)
183-185: Don’t send cursor: undefinedInitialize params only when a cursor exists to avoid API quirks.
- const params = { - cursor: this._getLastCursor(), - }; + const params = {}; + const lastCursor = this._getLastCursor(); + if (lastCursor != null) params.cursor = lastCursor;
190-190: Avoid hardcoded debug flagMake debug configurable (prop or env) to reduce noisy responses in prod.
- debug: true, + debug: !!process.env.PIPEDREAM_DEBUG,
196-206: Preserve this binding for payloadFilterPassing the method reference loses this if a source later uses instance state.
- const filteredPayloads = payloads.filter(this.payloadFilter); + const filteredPayloads = payloads.filter((p) => this.payloadFilter(p));
1-1: Package tip: avoid adding the npm “crypto” packageNode’s built-in crypto module suffices; depending on the npm “crypto” package can cause bundling conflicts. If package.json added crypto, remove it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
components/airtable_oauth/package.json(2 hunks)components/airtable_oauth/sources/common/common-webhook-field.mjs(0 hunks)components/airtable_oauth/sources/common/common-webhook-record.mjs(3 hunks)components/airtable_oauth/sources/common/common-webhook.mjs(7 hunks)components/airtable_oauth/sources/new-field/new-field.mjs(1 hunks)components/airtable_oauth/sources/new-modified-or-deleted-records-instant/new-modified-or-deleted-records-instant.mjs(3 hunks)components/airtable_oauth/sources/new-or-modified-field/new-or-modified-field.mjs(1 hunks)components/airtable_oauth/sources/new-or-modified-records/new-or-modified-records.mjs(2 hunks)components/airtable_oauth/sources/new-records/new-records.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- components/airtable_oauth/sources/common/common-webhook-field.mjs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/airtable_oauth/package.json
📚 Learning: 2024-07-24T02:06:47.016Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Applied to files:
components/airtable_oauth/sources/common/common-webhook-record.mjs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (11)
components/airtable_oauth/package.json (2)
3-3: Version bump looks good.Patch bump aligns with dependency additions.
18-18: Adding async-retry is appropriate for transient Airtable GET errors.No concerns.
components/airtable_oauth/sources/new-records/new-records.mjs (1)
8-8: Version bump OK.components/airtable_oauth/sources/new-or-modified-field/new-or-modified-field.mjs (1)
9-9: Version bump OK.components/airtable_oauth/sources/new-field/new-field.mjs (1)
8-8: Version bump OK.components/airtable_oauth/sources/new-modified-or-deleted-records-instant/new-modified-or-deleted-records-instant.mjs (2)
10-10: Version bump OK.
29-29: Good move: reuse common.props.airtable in propDefinition.Reduces duplicate app wiring.
components/airtable_oauth/sources/new-or-modified-records/new-or-modified-records.mjs (2)
27-27: Prop def source looks goodReferencing common.props.airtable keeps propDefinitions consistent with the shared base.
8-8: Version bump OKPatch version aligns with behavior-only changes.
components/airtable_oauth/sources/common/common-webhook.mjs (2)
131-134: Normalized default event shape looks goodWrapping in { originalPayload } addresses the “raw payload occasionally emitted” bug.
52-55: Persisting macSecretBase64 on activate is correctCapturing and storing the MAC secret from createWebhook enables downstream signature validation.
Also applies to: 82-83
WHY
Resolves 18220
Summary by CodeRabbit