Skip to content

fix: enable field removal for destination metadata/delivery_metadata#844

Open
alexluong wants to merge 3 commits intomainfrom
fix/destination-field-removal
Open

fix: enable field removal for destination metadata/delivery_metadata#844
alexluong wants to merge 3 commits intomainfrom
fix/destination-field-removal

Conversation

@alexluong
Copy link
Copy Markdown
Collaborator

@alexluong alexluong commented Apr 20, 2026

Destination Field Removal Fix

Problem

Impossible to clear filter, metadata, or delivery_metadata on destination update by sending {}.

  • Server: metadata/delivery_metadata used merge semantics — sending {} was a no-op
  • Go SDK: json:",omitempty" drops empty maps before serialization, so {} never reaches the server

Server Fix

Changed metadata and delivery_metadata from merge to full replacement (matching how filter already worked).

Input Behavior
Omitted / null No change
{} Clears the field
{"key": "val"} Replaces entirely

config and credentials intentionally keep merge semantics.

SDK Status

SDK Can send {} to clear? Breaking changes?
TypeScript ✅ Works None
Python ✅ Works None
Go ❌ No See below
Portal ✅ Works N/A

All 148 TS tests and 11 Python tests pass.

Go SDK Limitation

Go's omitempty strips empty maps before serialization — {} becomes indistinguishable from "not sent".

Speakeasy has a nullableOptionalWrapper flag that generates three-state OptionalNullable[T] types (not set vs null vs value). This would solve it, but the flag is global and all-or-nothing — it wraps ~30+ fields across the entire SDK (Create, Response, Event, Attempt, Tenant), not just the update structs.

For example, with the flag enabled, the update struct changes from:

// Before (current) — empty map is silently dropped by omitempty
type DestinationUpdateWebhook struct {
    Filter           map[string]any    `json:"filter,omitempty"`
    DeliveryMetadata map[string]string `json:"delivery_metadata,omitempty"`
    Metadata         map[string]string `json:"metadata,omitempty"`
}
// After (with flag) — three-state: not set, null, or value (including {})
type DestinationUpdateWebhook struct {
    Filter           optionalnullable.OptionalNullable[map[string]any]    `json:"filter,omitempty"`
    DeliveryMetadata optionalnullable.OptionalNullable[map[string]string] `json:"delivery_metadata,omitempty"`
    Metadata         optionalnullable.OptionalNullable[map[string]string] `json:"metadata,omitempty"`
}

This is what we want — but the same wrapping also applies to every other nullable field: Tenant.Metadata, Event.Metadata, Attempt.ResponseData, Attempt.Event, DestinationCreateWebhook.Filter, all response structs, etc. (~30+ fields total).

DX impact on Go SDK usage

Creating a destination (current):

sdk.Destinations.Create(ctx, tenantID, components.DestinationCreateWebhook{
    Type:   "webhook",
    Topics: components.TopicsArrayOfStrings([]string{"user.created"}),
    Config: components.WebhookConfig{URL: "https://example.com/webhook"},
    Metadata: map[string]string{"team": "platform"},
})

Creating a destination (with flag):

sdk.Destinations.Create(ctx, tenantID, components.DestinationCreateWebhook{
    Type:   "webhook",
    Topics: components.TopicsArrayOfStrings([]string{"user.created"}),
    Config: components.WebhookConfig{URL: "https://example.com/webhook"},
    Metadata: optionalnullable.Of(map[string]string{"team": "platform"}),
})

Clearing metadata on update (with flag):

sdk.Destinations.Update(ctx, tenantID, destID, components.DestinationUpdateWebhook{
    Metadata: optionalnullable.Of(map[string]string{}), // sends {} → clears
})

Reading from response (with flag):

dest, _ := sdk.Destinations.Get(ctx, tenantID, destID)
// Before: dest.Metadata["team"]
// After:  dest.Metadata.Value()["team"]  (must unwrap OptionalNullable first)

Every nullable field across the SDK requires wrapping/unwrapping — not just the update fields we care about.

What we tried to scope it to update fields only
  • Go-specific OpenAPI overlay converting only update fields to 3.1-style nullable (type: ["object", "null"]) — Speakeasy doesn't distinguish 3.0 vs 3.1, still wraps everything
  • Searched for per-field extension (x-speakeasy-nullable etc.) — doesn't exist in Speakeasy's 52 extensions

Note: Go SDK currently does not support sending null either — only omitted and value. This is a current Outpost limitation, not a design decision. If we want to support explicit null semantics in the future, we'd need to explore the OptionalNullable wrapper regardless.

Options:

  1. Document as Go limitation — users who need to clear fields use raw HTTP
  2. Accept breaking change — flip the flag, bump Go SDK major version
  3. Engage Speakeasy — request per-field nullable control

Test plan

  • Unit tests: 12 tests in destination_handlers_test.go (filter/metadata/delivery_metadata: replaced not merged, cleared with {}, unchanged when omitted)
  • SDK e2e tests (TS): 11 field removal tests + 148 total passing
  • SDK e2e tests (Python): 11 field removal tests passing
  • Pre-existing test fixes: events.test.ts and tenants.test.ts updated for PageIterator response shape

🤖 Generated with Claude Code

alexluong and others added 2 commits April 20, 2026 15:02
…ion update

Replace MergeStringMaps with direct assignment for metadata and
delivery_metadata, matching existing filter behavior. User-defined
key-value maps need full replacement semantics so keys can be removed
and the map can be cleared with {}.

Config and credentials still use merge since they have structured,
known keys where partial updates make sense.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lWrapper

SDK tests across TS, Go, and Python verifying that filter, metadata,
and delivery_metadata can be set, cleared with {}, have keys removed
via subset, and remain unchanged when omitted.

Also enables nullableOptionalWrapper in Go SDK gen config so the
generated SDK can distinguish "don't send" from "send empty" for
nullable map fields.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pagination endpoints return PageIterator<Response> where the body is
under .result. Update events.test.ts and tenants.test.ts to use
page.result.models instead of response.models.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants