Alanxtl commented on code in PR #3443:
URL: https://github.com/apache/dubbo-go/pull/3443#discussion_r3434057317
##########
protocol/triple/client.go:
##########
@@ -66,12 +66,18 @@ type clientManager struct {
// TODO: code a triple client between clientManager and triple_protocol client
// TODO: write a NewClient for triple client
-func (cm *clientManager) callUnary(ctx context.Context, method string, req,
resp any) error {
+func (cm *clientManager) callUnary(ctx context.Context, method string, req,
resp any, responseHeader, responseTrailer *http.Header) error {
triReq := tri.NewRequest(req)
triResp := tri.NewResponse(resp)
if err := cm.triClient.CallUnary(ctx, triReq, method, triResp); err !=
nil {
return err
}
+ if responseHeader != nil {
+ *responseHeader = triResp.Header().Clone()
+ }
+ if responseTrailer != nil {
+ *responseTrailer = triResp.Trailer().Clone()
Review Comment:
unary error response metadata 仍然不会回填到
`WithResponseHeader/WithResponseTrailer`
位置：[protocol/triple/client.go](/home/lxt/repo/dubbo-go/protocol/triple/client.go:72)
现在 `callUnary` 只有在 `cm.triClient.CallUnary(...)` 返回 nil 后才 copy
`triResp.Header()/Trailer()`。但底层 `receiveUnaryResponse` 在 `conn.Receive(...)`
返回 error 时会直接 return，还没把 `conn.ResponseHeader()/ResponseTrailer()` 写进
`triResp`。所以如果服务端 unary 返回 error，并携带 error metadata / trailers，用户传入的
`WithResponseHeader/WithResponseTrailer` 不会拿到任何值。
这会让新 API 在失败响应场景下不完整。RPC metadata 通常不只成功响应有价值，错误响应里的 trailer/header
也常用于诊断、quota、trace、业务错误附加信息等。建议补一个 error case 测试，并考虑在底层 unary receive 出错前后也尽量填充
response metadata，或者在文档/API 注释里明确该 option 只捕获成功响应 metadata。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
