feat(sqs): triggers own subscription config so spread needs no workarounds#405
Conversation
…ounds A trigger is not a plain consumer: it owns its handlers, its subscription filtering, and its lifecycle. Spreading a pre-resolved consumer-options object (e.g. @lokalise/aws-config resolveConsumerOptions) into a trigger source forced callers to hand-unset fields the resolver computed for a real consumer. Two fixes remove those workarounds: - AbstractSqsTrigger: only call the underlying consumer start() (which inits internally) instead of init() then start(). The previous double-init re-subscribed each queue, conflicting with subscription attributes (filter policy, redrive policy) applied on the first pass. - buildSnsTriggerConsumerOptions: centrally strip the fields a trigger owns from the spread, instead of trusting callers to override them at every call site: the handler-derived subscription filter policy (subscriptionConfig.Attributes, defaults to accept-all) and a spread-in subscriptionDeadLetterQueue (trigger manages failures via the queue-level deadLetterQueue). Shared by the flat and group SNS triggers; exported for reuse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughExtracts SNS→SQS consumer option normalization into a new exported ChangesSNS Trigger Consumer Options Refactor and Lifecycle Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
The double-init fix means a subscription DLQ no longer conflicts with the trigger lifecycle, so there is no reason to strip it. Narrow the trigger's subscription ownership to just the handler-derived filter policy: - Strip only Attributes.FilterPolicy / FilterPolicyScope (the reject-all policy resolveConsumerOptions derives from empty handlers), preserving every other subscription attribute — notably Attributes.RedrivePolicy, the SNS subscription-level DLQ in the installed toolkit. - Stop stripping a spread-in subscriptionDeadLetterQueue; let it flow through to the consumer (used by resolveConsumerOptions / newer toolkit versions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
Spreading a pre-resolved consumer-options object (e.g.
@lokalise/aws-config'sresolveConsumerOptions(...)) into a trigger source forced callers to hand-unset fields the resolver had computed for a real consumer. Real-world call sites looked like this:Root cause: a trigger is not a plain consumer — it owns its handlers and its subscription filtering — but it blindly spread a full consumer's options, leaving callers to un-set everything that didn''t apply.
Changes
A. Fix the double-init in
AbstractSqsTrigger. It calledc.init()andc.start(), but the underlyingmessage-queue-toolkitconsumer''sstart()already runsinit()internally — so every consumer initted twice, and the second pass re-subscribed each queue, conflicting with the subscription attributes (filter policy, redrive policy) set on the first pass. Now it only callsstart().B. The trigger claims only what it truly owns, centrally.
buildSnsTriggerConsumerOptionsresets:handlers— always rebuilt frombindings.subscriptionConfig.Attributes.FilterPolicy/FilterPolicyScope) — dropped (the reject-all policy the resolver derives from empty handlers); defaults to accept-all.Everything else flows through, including the subscription-level DLQ. Since fix A removes the double-init conflict, there''s no longer any reason to strip it:
subscriptionConfig.Attributes.RedrivePolicy(the subscription DLQ in the installed toolkit) is preserved alongside any other attributes, and a spread-insubscriptionDeadLetterQueue(used byresolveConsumerOptions/ newer toolkits) flows through to the consumer.Shared by the flat and group SNS triggers, and exported. SQS-queue triggers were already lifecycle-only and gain fix A for free.
After this, the call site is just
...resolvedOptions+creationConfig+bindings— nosubscriptionConfigpatch, nosubscriptionDeadLetterQueueoverride, noAttributesreset.Tests
New
test/triggerInternals.spec.ts(8 deterministic unit tests): the option sanitizer (handler injection,FilterPolicy/FilterPolicyScopedrop,RedrivePolicypreserved,subscriptionDeadLetterQueueforwarded,updateAttributesIfExistsdefault/override, pass-through of unrelated options) and the lifecycle fix (each consumerstart()ed exactly once,init()never called separately;stop()closes all). Full suite: 42 passed, no type errors.🤖 Generated with Claude Code