Repository navigation
fix: clear the lazy init promise on publisher close - #654
ihlokalise wants to merge 1 commit into
Conversation
SQS, SNS and Pub/Sub publishers kept their in-flight lazy init promise across close(), so a publish after close() could await an attempt that started before it. Clear it on close, as AbstractAmqpPublisher already does.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Reset initialization on close and verify later publish packages/gcp-pubsub/lib/pubsub/AbstractPubSubPublisher.ts, packages/gcp-pubsub/test/publishers/PubSubPermissionPublisher.spec.ts, packages/sns/lib/sns/AbstractSnsPublisher.ts, packages/sns/test/publishers/SnsPermissionPublisher.spec.ts, packages/sqs/lib/sqs/AbstractSqsPublisher.ts, packages/sqs/test/publishers/SqsPermissionPublisher.spec.ts |
Each publisher clears initPromise before calling super.close(). Tests verify that a publish after close triggers a second initialization and succeeds. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Possibly related PRs
- kibertoad/message-queue-toolkit#653: This PR and PR
#653change reset conditions for the lazy-initialization promise in the SQS, SNS, and Pub/Sub publishers.
Suggested reviewers: kjamrog, carlosgamero
Merge Risk: 
Minimal · up to e5df6
-
The change is ready to merge after normal checks; no actionable merge-blocking risk was established.
Pre-merge checks: 
5
-
Full details: Title check
-
Explanation
The title "fix: clear the lazy init promise on publisher close" matches the main change. The PR clears the pending lazy-initialization promise in close() for the SQS, SNS, and Pub/Sub publishers, which is the core fix. The title is short, specific, and states the primary change from the developer's view. The test additions and the AMQP alignment are supporting details that the title does not need to cover.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 6 files.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Follow-up to #653, from @CarlosGamero's review comment there.
In plain words
When
publish()is called before the publisher is ready, it starts getting ready (the "lazy init") and keeps a note of that attempt, so other publishes can wait on the same attempt instead of starting their own.close()in the SQS, SNS and Pub/Sub publishers didn't throw that note away. Picture a publisher that's halfway through getting ready when it gets closed. If you publish again afterwards, it finds the old note and waits on the attempt that started before the close, instead of getting ready again from scratch. If that old attempt is slow or never finishes, the new publish waits with it.The AMQP publisher already throws the note away in
close(). This PR does the same in the other three, so all four behave alike.What changes
AbstractSqsPublisher,AbstractSnsPublisherandAbstractPubSubPublisherget the sameclose()thatAbstractAmqpPublisheralready has:What doesn't change
Publishing after
close()works exactly as it does today:isInittedisfalse, so the nextpublish()runs the lazy init again and sends. Nothing is rejected, and publishers initialised up front withinit()aren't affected.Tests
One test per package. A lazy init is left hanging, then the publisher is closed and
publish()is called again. The test checks that a fresh init runs and the message is delivered.Without the fix, all three tests hang until they time out, because the second publish waits forever on the old attempt. With the fix they pass. The full publisher suites for
sqs,snsandgcp-pubsubpass, and lint (biome+tsc) is clean.Summary by CodeRabbit