feat(ITcpSocketClient): add ReceivedCallback property #6266
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideImplements a ReceivedCallback on TCP clients and data handlers to allow consumers to process raw inbound bytes directly; updates default client, interface, handler base, samples, and tests to support and demonstrate this new callback. Sequence diagram for data reception with ReceivedCallBack in DefaultTcpSocketClientsequenceDiagram
participant Client as DefaultTcpSocketClient
participant Network as Network
participant UserCallback as ReceivedCallBack
participant Handler as DataPackageHandlerBase
Network->>Client: Data arrives
Client->>Client: ReceiveAsync()
alt ReceivedCallBack is set
Client->>UserCallback: Invoke ReceivedCallBack(buffer)
end
alt DataPackageHandler is set
Client->>Handler: ReceiveAsync(buffer)
end
Class diagram for ReceivedCallBack integration in TCP socket clientclassDiagram
class ITcpSocketClient {
+IPEndPoint LocalEndPoint
+Func<ReadOnlyMemory<byte>, ValueTask>? ReceivedCallBack
+SetDataHandler()
+SendAsync()
+Close()
}
class DefaultTcpSocketClient {
+Func<ReadOnlyMemory<byte>, ValueTask>? ReceivedCallBack
+ReceiveAsync()
}
class DataPackageHandlerBase {
+Func<ReadOnlyMemory<byte>, ValueTask>? ReceivedCallBack
}
ITcpSocketClient <|.. DefaultTcpSocketClient
IDataPackageHandler <|.. DataPackageHandlerBase
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Services/TcpSocket/DefaultTcpSocketClient.cs:31` </location>
<code_context>
+ /// <remarks>The callback function should be designed to handle the received data efficiently and
+ /// asynchronously. Ensure that the implementation does not block or perform long-running operations, as this may
+ /// impact performance.</remarks>
public Func<ReadOnlyMemory<byte>, ValueTask>? ReceivedCallBack { get; set; }
/// <summary>
</code_context>
<issue_to_address>
Consider thread-safety for ReceivedCallBack property.
If this client may be used by multiple threads, ensure ReceivedCallBack is accessed in a thread-safe manner or clearly document that it must not be modified after connection.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Services/TcpSocket/DefaultTcpSocketClient.cs:141` </location>
<code_context>
{
buffer = buffer[..len];
+ if (ReceivedCallBack != null)
+ {
+ await ReceivedCallBack(buffer);
+ }
+
</code_context>
<issue_to_address>
Handle exceptions from ReceivedCallBack invocation.
Wrap the callback in a try-catch block to prevent unhandled exceptions from breaking the receive loop, and consider logging or handling errors appropriately.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if (ReceivedCallBack != null)
{
await ReceivedCallBack(buffer);
}
=======
if (ReceivedCallBack != null)
{
try
{
await ReceivedCallBack(buffer);
}
catch (Exception ex)
{
Console.Error.WriteLine($"Exception in ReceivedCallBack: {ex}");
// Optionally, handle or rethrow as needed
}
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/BootstrapBlazor.Server/Components/Samples/SocketFactories.razor:25` </location>
<code_context>
<li>通过 <code>ITcpSocketClient</code> 实例方法 <code>SendAsync</code> 发送协议数据</li>
<li>通过 <code>ITcpSocketClient</code> 实例方法 <code>Close</code> 关闭连接</li>
<li>通过 <code>ITcpSocketClient</code> 实例方法 <code>SetDataHandler</code> 方法设置数据处理器</li>
+ <li>通过 <code>ITcpSocketClient</code> 实例属性 <code>ReceivedCallBack</code> 方法设置接收数据处理器（注意：此回调未做任何数据处理为原始数据）</li>
</ul>
</code_context>
<issue_to_address>
Clarify the distinction between ReceivedCallBack and SetDataHandler in documentation.
Consider specifying when to use ReceivedCallBack versus SetDataHandler to help users choose the appropriate method.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
<li>通过 <code>ITcpSocketClient</code> 实例方法 <code>SetDataHandler</code> 方法设置数据处理器</li>
<li>通过 <code>ITcpSocketClient</code> 实例属性 <code>ReceivedCallBack</code> 方法设置接收数据处理器（注意：此回调未做任何数据处理为原始数据）</li>
</ul>
=======
<li>通过 <code>ITcpSocketClient</code> 实例方法 <code>SetDataHandler</code> 方法设置数据处理器。<br />
<b>适用场景：</b>推荐用于需要对接收到的数据进行协议解析、粘包/分包处理等高级数据处理的场景。数据会经过处理后再传递给回调函数，适合大多数业务开发需求。
</li>
<li>通过 <code>ITcpSocketClient</code> 实例属性 <code>ReceivedCallBack</code> 方法设置接收数据处理器（注意：此回调未做任何数据处理，为原始数据）。<br />
<b>适用场景：</b>适合需要直接获取原始字节流数据、无需协议解析或自定义底层数据处理的场景。<br />
<b>区别说明：</b><br />
<ul>
<li><code>ReceivedCallBack</code> 直接返回原始数据，适合底层自定义处理。</li>
<li><code>SetDataHandler</code> 经过协议处理，适合业务层直接使用。</li>
</ul>
<b>建议：</b>如无特殊需求，优先使用 <code>SetDataHandler</code> 以简化开发流程。
</li>
</ul>
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `test/UnitTest/Services/TcpSocketFactoryTest.cs:164` </location>
<code_context>
var data = new ReadOnlyMemory<byte>([1, 2, 3, 4, 5]);
await client.SendAsync(data);
+ await tcs.Task;
+ Assert.Equal(buffer.ToArray(), [1, 2, 3, 4, 5]);
+
</code_context>
<issue_to_address>
Potential for test hang if callback is not invoked.
Add a timeout to the TaskCompletionSource await (e.g., Task.WhenAny with a timeout) to avoid indefinite hangs and improve test diagnostics.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
await tcs.Task;
Assert.Equal(buffer.ToArray(), [1, 2, 3, 4, 5]);
=======
var completedTask = await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(5)));
if (completedTask != tcs.Task)
{
throw new TimeoutException("Timed out waiting for ReceivedCallBack to be invoked.");
}
Assert.Equal(buffer.ToArray(), [1, 2, 3, 4, 5]);
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6266 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 711 711
Lines 31374 31379 +5
Branches 4431 4432 +1
=========================================
+ Hits 31374 31379 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6265
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add ReceivedCallBack for raw data events on TCP socket client and integrate support in implementation, documentation, and tests.
New Features:
Documentation:
Tests: