FISCO-BCOS / go-sdk

golang SDK of FISCO BCOS
Apache License 2.0
73 stars 58 forks source link

fix: already subscribe to topic #231

Closed liuxinfeng96 closed 1 year ago

liuxinfeng96 commented 1 year ago

修复在使用SubscribeBlockNumberNotify()时候,订阅失败的问题。 原因分析: 在客户端与节点拨号连接的时候,该代码注释的位置导致_blocknotify[groupId]这个topic注册了,后面使用客户端调用SubscribeBlockNumberNotify函数注册新的处理器并不能注册成功,而且由于这里的处理器注册是个nil,即使先取消订阅,UnSubscribeBlockNumberNotify,也会失败。

bxq2011hust commented 1 year ago

看了下代码,这里应该是符合预期的,实际的处理函数存储在blockNotifyHandlers 可以阅读下这里的代码 https://github.com/FISCO-BCOS/go-sdk/blob/584756b7b270d2be71325efbcca3f82d0c4cb1fc/conn/channel.go#L1229-L1295

如果还是有问题,请提供下重现方式,以便于我们跟进

liuxinfeng96 commented 1 year ago

您说得符合预期是指什么呢?如果不注释掉这段代码的话,我看到的现象是这样的:我使用DialContext()函数建立客户端后,然后去订阅区块高度的事件通知是失败的。订阅高度的事件通知使用的是这个函数:SubscribeBlockNumberNotify()。 我的链版本:v2.9.1 go sdk 分支:master-FISCO-BCOS-v2 您可以尝试复现一下。

bxq2011hust commented 1 year ago

您说得符合预期是指什么呢?如果不注释掉这段代码的话,我看到的现象是这样的:我使用DialContext()函数建立客户端后,然后去订阅区块高度的事件通知是失败的。订阅高度的事件通知使用的是这个函数:SubscribeBlockNumberNotify()。 我的链版本:v2.9.1 go sdk 分支:master-FISCO-BCOS-v2 您可以尝试复现一下。

谢谢反馈,我重现了你描述的问题。但不能直接注释PR中的代码,这样改会导致sdk没法拿到块高推送。建议按照下面的方式修改

image
    if oldHandler, ok := hc.topicHandlers[topic]; ok && oldHandler != nil {
        return errors.New("already subscribed to topic " + topic)
    }
liuxinfeng96 commented 1 year ago

好的,我那样改确实太粗暴了,因为我并没有理解那段代码的含义,您的改法是正确的,不会影响代码原逻辑。

codecov[bot] commented 1 year ago

Codecov Report

Patch has no changes to coverable lines.

:loudspeaker: Thoughts on this report? Let us know!.