Skip to content

Conversation

@torcolvin
Copy link
Collaborator

CBG-4928 do not log warning for closed blip sender

This should prevent a log message as below when a blip connection is closed and it can not longer send changes. I believe a blip connection could be closed due to normal network disconnections:

[WRN] c:[231d979a] db:db col:col2 Error from bh.handleChangesResponse: use of closed BLIP sender -- db.(*blipHandler).sendBatchOfChanges.func1() at blip_handler.go:614

It is OK to not enter fatalErrorCallback

apr.blipSyncContext.fatalErrorCallback = func(err error) {
if strings.Contains(err.Error(), ErrUseProposeChanges.Message) {
since if the sender is closed it will enter https://github.com/couchbase/sync_gateway/blob/main/db/active_replicator.go#L238 which will close and reconnect.

This was hard to unit test, but is observed in real world logs, so pushing this change to avoid unexpected logs. In order to test this, I believe you have to have the sender closed at the right time.

NOTE: this is only one part of CBG-4928 and there is a second part needed to address:

[WRN] c:[393a64c4] db:db col:col1 Error retrieving changes for channel "<ud>b2</ud>": Changes feed cancelled while waiting for view lock -- db.(*DatabaseCollectionWithUser).changesFeed.func1() at changes.go:437
[WRN] c:[393a64c4] db:db col:col1 MultiChangesFeed got error reading changes feed: Error while building channel feed -- db.(*DatabaseCollectionWithUser).SimpleMultiChangesFeed.func1() at changes.go:914

Copilot AI review requested due to automatic review settings November 13, 2025 17:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a false alarm in logging when BLIP connections close normally during replication by filtering out expected errors from closed BLIP senders.

  • Prevents warning logs when BLIP sender closes during normal network disconnections
  • Adds error filtering to avoid invoking fatal error callbacks for expected closed sender errors

// Spawn a goroutine to await the client's response:
go func(bh *blipHandler, sender *blip.Sender, response *blip.Message, changeArray [][]any, sendTime time.Time, dbCollection *DatabaseCollectionWithUser) {
if err := bh.handleChangesResponse(bh.loggingCtx, sender, response, changeArray, sendTime, dbCollection, bh.collectionIdx); err != nil {
if err := bh.handleChangesResponse(bh.loggingCtx, sender, response, changeArray, sendTime, dbCollection, bh.collectionIdx); err != nil && !errors.Is(err, ErrClosedBLIPSender) {
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

[nitpick] The condition is now complex with multiple checks on a single line. Consider extracting the error check to a separate variable for improved readability: if err := bh.handleChangesResponse(...); err != nil { if !errors.Is(err, ErrClosedBLIPSender) { ... } }

Copilot uses AI. Check for mistakes.
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.

4 participants