Skip to content

fix(pubsub): ensure listener processes all messages#36

Merged
ren0503 merged 1 commit into
masterfrom
fix/ren/35-fix-receive-message-once
Jan 27, 2026
Merged

fix(pubsub): ensure listener processes all messages#36
ren0503 merged 1 commit into
masterfrom
fix/ren/35-fix-receive-message-once

Conversation

@ren0503

@ren0503 ren0503 commented Jan 27, 2026

Copy link
Copy Markdown
Contributor
  • Fix issue where listener would only process the first message.
  • Add unit test to verify 100 messages are received.

@ren0503 ren0503 added this to the Pubsub Release v2.2.1 milestone Jan 27, 2026
@ren0503 ren0503 linked an issue Jan 27, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Jan 27, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • New Features

    • Pub/sub now continuously processes multiple incoming messages per invocation.
  • Tests

    • Added a test validating delivery of many concurrent pub/sub messages.
  • Chores

    • Bumped a dependency version.
    • CI workflow simplified so tests and coverage upload run unconditionally.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Dependency bumped; listener refactored from single receive to range-loop to process all messages; new integration test publishes 100 messages and verifies receipt; CI workflow removed conditional gating so tests and coverage upload run unconditionally.

Changes

Cohort / File(s) Summary
Dependency Update
go.mod
Updated github.com/tinh-tinh/tinhtinh/v2 requirement from v2.3.4 to v2.5.1
Message Processing Logic
listener.go
Replaced single receive-with-ok branch with for range over sub.GetMessages(), processing each message until channel closes
Integration Tests
listener_test.go
Added Test_Listener_MultipleMessages: publishes 100 messages to topic, uses WaitGroup and 1s timeout, asserts 100 receipts
CI Workflow
.github/workflows/go.yml
Removed conditional "Decide if tests should run" step; test and coverage steps now unconditional

Sequence Diagram(s)

sequenceDiagram
    participant Pub as Publisher
    participant Broker as Broker / Channel
    participant L as Listener
    participant H as Handler/Factory

    rect rgba(0, 100, 200, 0.5)
    Note over L: Old pattern (single receive)
    Pub->>Broker: send message
    L->>Broker: receive (single)
    Broker-->>L: msg, ok
    alt ok == false
        L-->>L: exit
    end
    L->>H: factory(msg)
    H-->>L: processed
    end

    rect rgba(100, 200, 0, 0.5)
    Note over L: New pattern (continuous range)
    Pub->>Broker: send message 1
    Pub->>Broker: send message 2
    Pub->>Broker: send message 3
    L->>Broker: range over messages
    loop for each message
        Broker-->>L: msg
        L->>H: factory(msg)
        H-->>L: processed
    end
    Pub->>Broker: close channel
    Broker-->>L: channel closed
    L-->>L: exit range
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I hopped where messages once paused,
Now I range and chew the cause—
A hundred leaves fall in a row,
I count each bite as onward go.
Hooray for channels, swift and bright!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely describes the main fix: ensuring the listener processes all messages instead of just the first one, which aligns with the core change in listener.go.
Description check ✅ Passed The PR description directly relates to the changeset by explaining the bug fix (listener processing only first message) and the added test, matching the modifications across listener.go and listener_test.go.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@listener_test.go`:
- Around line 90-96: The timeout branch in the select (waiting on the done
channel and time.After(1 * time.Second)) only prints a message, so change it to
fail the test by calling the testing.T failure method (e.g., replace
fmt.Println("Timeout waiting for messages") with t.Fatalf("timeout waiting for
messages after %s", time.Second) or t.Fatal("timeout waiting for messages")) so
a missed SLA causes the test to fail; keep the select using the same done
channel and time.After call.
- Around line 3-13: The test’s receivedCount is updated in the listener
goroutine and read in the main test goroutine, causing a race; change
receivedCount to an int32/int64 and use sync/atomic.AddInt32/Int64 in the
listener and sync/atomic.LoadInt32/LoadInt64 for the final assertion
(references: receivedCount, the listener goroutine that increments it, and the
test assertion that reads it). Also, replace the current timeout branch that
only prints with a test failure (t.Fatalf or require.Fail) so the timeout path
fails the test explicitly. Ensure imports include sync/atomic and adjust
assertions to compare the atomic-loaded value.

Comment thread listener_test.go
Comment thread listener_test.go
- Fix issue where listener would only process the first message.
- Add unit test to verify 100 messages are received.
@ren0503 ren0503 force-pushed the fix/ren/35-fix-receive-message-once branch from 8156f8c to 8476666 Compare January 27, 2026 14:52
@codecov

codecov Bot commented Jan 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ren0503 ren0503 merged commit 9b0b7b2 into master Jan 27, 2026
3 checks passed
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.

Listener Only Receives One Message

1 participant