Conversation
- Add WebhookPayload types matching official Inbound documentation - Support email.received webhook events with nested data structures - Add ParseWebhookPayload() function for parsing webhook requests - Add helper methods: GetFromAddress(), GetToAddress(), GetHeaders() - Support both parsedData and cleanedContent from webhook payloads - Handle complex header parsing (strings, arrays, objects) - Add comprehensive webhook parsing tests with edge cases - Update types to match official webhook structure - Support flexible date handling and optional fields This enables applications to properly parse Inbound webhook payloads instead of relying on custom parsing logic. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-116df522-92b1-4a32-9d68-11ba56225ea2
WalkthroughIntroduces inbound webhook support: new types modeling the webhook payload, a parser to decode JSON into these structures, helper methods to access addresses and headers, update to CHANGELOG, and comprehensive tests validating parsing, helpers, headers, cleaned content, and endpoint metadata. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Webhook Sender
participant Handler as ParseWebhookPayload
participant JSON as JSON Decoder
participant Model as WebhookPayload
Client->>Handler: POST body (io.Reader)
Handler->>JSON: Decode into WebhookPayload
JSON-->>Handler: Parsed struct or error
alt decode ok
Handler->>Client: *WebhookPayload
note over Model,Handler: Accessors<br/>- GetFromAddress<br/>- GetToAddress<br/>- GetHeaders
Client->>Model: GetFromAddress()
Model-->>Client: "Name <email>" or "email" or ""
Client->>Model: GetToAddress()
Model-->>Client: "Name <email>" or "email" or ""
Client->>Model: GetHeaders()
Model-->>Client: map[string][]string
else decode error
Handler-->>Client: error (wrapped with context)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
CHANGELOG.md (2)
10-12: Release date looks inconsistent with this PR timeline0.2.0 is dated 2025-01-16, but this PR was opened on 2025-09-16. Either keep this section under “Unreleased” or update the date to the actual release date.
29-30: Clarify “Date object” wording for GoConsider phrasing as “ISO‑8601 string (RFC3339/RFC3339Nano) or provider-specific object” to avoid implying a JavaScript Date. If you intend to support non‑string encodings, link to the provider doc.
webhook.go (3)
20-30: Harden From-address extraction for sparse payloadsIf the first entry has an empty Address, consider scanning for the first non‑empty Address and, if none exist, falling back to group Text when non‑empty.
Apply this diff:
func (w *WebhookPayload) GetFromAddress() string { - if len(w.Email.From.Addresses) > 0 { - addr := w.Email.From.Addresses[0] - if addr.Name != nil && *addr.Name != "" { - return fmt.Sprintf("%s <%s>", *addr.Name, addr.Address) - } - return addr.Address - } - return "" + for _, addr := range w.Email.From.Addresses { + if addr.Address == "" { + continue + } + if addr.Name != nil && *addr.Name != "" { + return fmt.Sprintf("%s <%s>", *addr.Name, addr.Address) + } + return addr.Address + } + if w.Email.From.Text != "" { + return w.Email.From.Text + } + return "" }
32-42: Mirror the From logic for To-addressSame improvement applies to To: scan for first non‑empty address; fallback to Text.
func (w *WebhookPayload) GetToAddress() string { - if len(w.Email.To.Addresses) > 0 { - addr := w.Email.To.Addresses[0] - if addr.Name != nil && *addr.Name != "" { - return fmt.Sprintf("%s <%s>", *addr.Name, addr.Address) - } - return addr.Address - } - return "" + for _, addr := range w.Email.To.Addresses { + if addr.Address == "" { + continue + } + if addr.Name != nil && *addr.Name != "" { + return fmt.Sprintf("%s <%s>", *addr.Name, addr.Address) + } + return addr.Address + } + if w.Email.To.Text != "" { + return w.Email.To.Text + } + return "" }
44-73: Normalize header keys and preserve non-string values instead of dropping them
- Email header names are case-insensitive; canonicalize keys.
- Don’t drop non-string values in []any or map[string]any. Fallback to fmt.Sprint or JSON encode the object when no “text”/“value” field exists.
- Optional: if ParsedData.Headers is empty, fall back to CleanedContent.Headers.
Apply this diff:
@@ -import ( +import ( "encoding/json" "fmt" "io" + "net/textproto" ) @@ -func (w *WebhookPayload) GetHeaders() map[string][]string { - headers := make(map[string][]string) - for k, v := range w.Email.ParsedData.Headers { +func (w *WebhookPayload) GetHeaders() map[string][]string { + headers := make(map[string][]string, len(w.Email.ParsedData.Headers)) + src := w.Email.ParsedData.Headers + if (src == nil || len(src) == 0) && w.Email.CleanedContent != nil && w.Email.CleanedContent.Headers != nil { + src = w.Email.CleanedContent.Headers + } + for k, v := range src { + kk := textproto.CanonicalMIMEHeaderKey(k) switch val := v.(type) { case string: - headers[k] = []string{val} + headers[kk] = []string[ ]{val} case []string: - headers[k] = val + headers[kk] = val case []any: - var strSlice []string + var strSlice []string for _, item := range val { - if str, ok := item.(string); ok { - strSlice = append(strSlice, str) - } + switch it := item.(type) { + case string: + strSlice = append(strSlice, it) + default: + strSlice = append(strSlice, fmt.Sprint(it)) + } } if len(strSlice) > 0 { - headers[k] = strSlice + headers[kk] = strSlice } case map[string]any: // Handle complex header structures like dkim-signature if text, ok := val["text"].(string); ok { - headers[k] = []string{text} + headers[kk] = []string{text} } else if value, ok := val["value"].(string); ok { - headers[k] = []string{value} + headers[kk] = []string{value} + } else { + if b, err := json.Marshal(val); err == nil { + headers[kk] = []string{string(b)} + } } + default: + headers[kk] = []string{fmt.Sprint(val)} } } return headers }webhook_test.go (3)
126-135: Consider additional To-address formatting caseAdd a test where the To name is present to assert “Name ” formatting.
167-180: Assert DKIM header value, not just presenceSince GetHeaders maps objects via “value”/“text”, assert the first value equals "v=1".
- if _, exists := headers["dkim-signature"]; !exists { - t.Error("Expected dkim-signature header to be parsed") - } + if got, exists := headers["Dkim-Signature"]; !exists || len(got) == 0 || got[0] != "v=1" { + t.Errorf("Expected dkim-signature header 'v=1', got %#v", headers["Dkim-Signature"]) + }
231-264: Edge case: empty From/ToLooks good. If you adopt fallback-to-Text logic, add a companion test where Text is non-empty but Addresses is empty.
types.go (4)
571-581: ReceivedAt as string: offer ergonomic time parsing helpersKeep JSON as string, but consider helper methods:
- WebhookEmailData.ReceivedAtTime() (time.Time, error)
- WebhookParsedData.DateTime() (time.Time, bool)
This adds type-safety without breaking the wire model.
Example:
type WebhookEmailData struct { @@ - ReceivedAt string `json:"receivedAt"` + ReceivedAt string `json:"receivedAt"` @@ } + +func (e WebhookEmailData) ReceivedAtTime() (time.Time, error) { + // Try RFC3339Nano then RFC3339 + if t, err := time.Parse(time.RFC3339Nano, e.ReceivedAt); err == nil { + return t, nil + } + return time.Parse(time.RFC3339, e.ReceivedAt) +}type WebhookParsedData struct { @@ - Date any `json:"date"` // Can be string or Date object + Date any `json:"date"` // string or provider-specific object @@ } + +func (p WebhookParsedData) DateTime() (time.Time, bool) { + switch v := p.Date.(type) { + case string: + if t, err := time.Parse(time.RFC3339Nano, v); err == nil { + return t, true + } + if t, err := time.Parse(time.RFC3339, v); err == nil { + return t, true + } + } + return time.Time{}, false +}
593-609: “any” for Date trades off safety; consider a dedicated FlexibleTime type (optional)If you prefer stronger typing, replace “any” with a json.Unmarshaler that accepts string/number/object encodings.
- Date any `json:"date"` // Can be string or Date object + Date FlexibleTime `json:"date"`Additional code (outside this hunk):
// FlexibleTime accepts RFC3339(/Nano) strings or UNIX seconds/millis. type FlexibleTime struct{ time.Time } func (ft *FlexibleTime) UnmarshalJSON(b []byte) error { // Try string var s string if err := json.Unmarshal(b, &s); err == nil { if t, err := time.Parse(time.RFC3339Nano, s); err == nil { ft.Time = t; return nil } if t, err := time.Parse(time.RFC3339, s); err == nil { ft.Time = t; return nil } } // Try number (seconds or millis) var n float64 if err := json.Unmarshal(b, &n); err == nil { sec := int64(n) if n > 1e12 { // millis heuristic ft.Time = time.UnixMilli(int64(n)) } else { ft.Time = time.Unix(sec, 0) } return nil } // Fallback: parse objects with "iso" or "value" string fields var m map[string]any if err := json.Unmarshal(b, &m); err == nil { if iso, ok := m["iso"].(string); ok { if t, err := time.Parse(time.RFC3339Nano, iso); err == nil { ft.Time = t; return nil } } if val, ok := m["value"].(string); ok { if t, err := time.Parse(time.RFC3339Nano, val); err == nil { ft.Time = t; return nil } } } return fmt.Errorf("unsupported time format") }
620-625: Attachment optionalityAre ContentType and ContentID guaranteed? If not, make them pointers with omitempty to distinguish absent vs empty.
type WebhookAttachment struct { Filename string `json:"filename"` - ContentType string `json:"contentType"` - ContentID string `json:"contentId"` + ContentType *string `json:"contentType,omitempty"` + ContentID *string `json:"contentId,omitempty"` URL string `json:"url"` }
583-591: Nit: grouping typesConsider moving webhook types into a dedicated file (e.g., webhook_types.go) to improve discoverability in this large types.go.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)types.go(1 hunks)webhook.go(1 hunks)webhook_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
webhook_test.go (1)
webhook.go (1)
ParseWebhookPayload(10-18)
webhook.go (1)
types.go (1)
WebhookPayload(564-569)
⏰ 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). (2)
- GitHub Check: test (1.21.x, windows-latest)
- GitHub Check: test (1.22.x, windows-latest)
🔇 Additional comments (4)
webhook.go (1)
10-18: Parser looks goodStraightforward decode with wrapped error context. No concerns.
webhook_test.go (2)
8-99: Solid end-to-end parse/fields assertionsGood coverage of top-level and nested fields, plus helpers and cleaned content.
200-229: Nice edge case: From without nameGood expectation and minimal payload. LGTM.
types.go (1)
563-569: Webhook payload root shape matches testsEvent/Timestamp/Email with optional Endpoint looks correct.
1 participant
This enables applications to properly parse Inbound webhook payloads instead of relying on custom parsing logic.
Amp-Thread-ID: https://ampcode.com/threads/T-116df522-92b1-4a32-9d68-11ba56225ea2
Summary by CodeRabbit
New Features
Tests
Documentation