This is an automated email from the ASF dual-hosted git repository.
Alanxtl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new ffbbcc3d9 docs(triple): complete Triple Header/Error comments and unit
test coverage (#3649)
ffbbcc3d9 is described below
commit ffbbcc3d9998c8463d7410bcf1304fd5026fe248
Author: Li Zining <[email protected]>
AuthorDate: Thu Aug 13 09:10:33 2026 +0800
docs(triple): complete Triple Header/Error comments and unit test coverage
(#3649)
* docs(triple): add doc comments for header, trailer and error exported APIs
Several exported APIs in the triple protocol lacked doc comments or had
incomplete ones. Add a doc comment for ExtractFromOutgoingContext
describing what it returns and how the framework uses it internally:
on the client side to populate request headers, and on the server side
to merge handler-set headers into response trailers. Augment the
SetHeader, SetTrailer and SendHeader comments to state that they return
a CodeInternal error when called outside a Triple handler context. Add
a doc comment for Error.Error describing its format.
Signed-off-by: lizining <[email protected]>
* test(triple): add boundary tests for header, trailer and error APIs
Add 13 tests covering previously untested branches and edge cases:
header_test.go (8 tests):
- DecodeBinaryHeader: empty, unpadded, padded and naturally-aligned inputs
- ExtractFromOutgoingContext: nil return when no headers set, correct
return when headers exist
- NewOutgoingContext: second call replaces existing headers instead of
merging
- AppendToOutgoingContext: panics on odd number of kv arguments
- SetHeader/SetTrailer: normal path merges headers into the response
buffer
- SetHeader/SetTrailer outside a handler context: returns CodeInternal
error_test.go (5 tests):
- NewWireError + IsWireError: wire, plain, non-triple and wrapped cases
- NewErrorDetail with *anypb.Any: skips double wrapping
- ErrorDetail.Bytes: returns a copy, mutation does not affect internal
state
- wrapIfContextError: Canceled, DeadlineExceeded, already-coded, plain
error and nil
- wrapIfUncoded: nil, already-coded, context.Canceled and plain error
Signed-off-by: lizining <[email protected]>
* style(triple): silence staticcheck SA5012 in the odd-kv panic test
The test deliberately passes a single kv argument to
AppendToOutgoingContext to verify it panics on an odd number of
arguments. staticcheck SA5012 flags the variadic call as having an odd
element count, which fails the CI lint. Annotate the call with
//nolint:staticcheck, following the existing convention in
handler_compat.go and handler_stream_compat.go.
Signed-off-by: lizining <[email protected]>
---------
Signed-off-by: lizining <[email protected]>
---
protocol/triple/triple_protocol/error.go | 2 +
protocol/triple/triple_protocol/error_test.go | 101 +++++++++++++++++++
protocol/triple/triple_protocol/header.go | 14 ++-
protocol/triple/triple_protocol/header_test.go | 133 +++++++++++++++++++++++++
4 files changed, 249 insertions(+), 1 deletion(-)
diff --git a/protocol/triple/triple_protocol/error.go
b/protocol/triple/triple_protocol/error.go
index 5e7cbbbd0..b2b983080 100644
--- a/protocol/triple/triple_protocol/error.go
+++ b/protocol/triple/triple_protocol/error.go
@@ -159,6 +159,8 @@ func IsWireError(err error) bool {
return se.wireErr
}
+// Error implements the [error] interface. It returns the status code's name
+// and, if non-empty, the underlying error's message separated by a colon.
func (e *Error) Error() string {
message := e.Message()
if message == "" {
diff --git a/protocol/triple/triple_protocol/error_test.go
b/protocol/triple/triple_protocol/error_test.go
index 70893654d..7b2615d67 100644
--- a/protocol/triple/triple_protocol/error_test.go
+++ b/protocol/triple/triple_protocol/error_test.go
@@ -18,6 +18,7 @@
package triple_protocol
import (
+ "context"
"errors"
"fmt"
"strings"
@@ -28,6 +29,7 @@ import (
import (
"google.golang.org/protobuf/proto"
+ "google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/durationpb"
"google.golang.org/protobuf/types/known/emptypb"
)
@@ -116,3 +118,102 @@ func TestErrorIs(t *testing.T) {
assert.False(t, errors.Is(tripleErr, NewError(CodeUnavailable, err)))
assert.True(t, errors.Is(tripleErr, tripleErr))
}
+
+// TestNewWireError verifies that IsWireError returns true for wire errors
+// created by NewWireError (including when wrapped) and false for regular
+// Triple errors and non-Triple errors.
+func TestNewWireError(t *testing.T) {
+ t.Parallel()
+ // Wire error is detected by IsWireError
+ wireErr := NewWireError(CodeUnavailable, errors.New("server down"))
+ assert.True(t, IsWireError(wireErr))
+ // Regular error is not a wire error
+ plainErr := NewError(CodeUnavailable, errors.New("client issue"))
+ assert.False(t, IsWireError(plainErr))
+ // Non-triple error is not a wire error
+ assert.False(t, IsWireError(errors.New("not triple")))
+ // Wrapped wire error is still detected
+ wrapped := fmt.Errorf("wrapped: %w", wireErr)
+ assert.True(t, IsWireError(wrapped))
+}
+
+// TestNewErrorDetailWithAny verifies that NewErrorDetail uses an *anypb.Any
+// directly without wrapping it into another Any.
+func TestNewErrorDetailWithAny(t *testing.T) {
+ t.Parallel()
+ // When msg is already an *anypb.Any, it should be used directly
+ // without wrapping into another Any.
+ anyMsg := &anypb.Any{
+ TypeUrl: "type.googleapis.com/google.protobuf.Empty",
+ Value: []byte{},
+ }
+ detail, err := NewErrorDetail(anyMsg)
+ assert.Nil(t, err)
+ assert.Equal(t, detail.Type(), "google.protobuf.Empty")
+}
+
+// TestErrorDetailBytesReturnsCopy verifies that ErrorDetail.Bytes returns a
+// copy of the serialized detail and that mutating it does not affect the
+// internal state.
+func TestErrorDetailBytesReturnsCopy(t *testing.T) {
+ t.Parallel()
+ detail, err := NewErrorDetail(durationpb.New(time.Second))
+ assert.Nil(t, err)
+ first := detail.Bytes()
+ assert.NotZero(t, first)
+ // Mutate the returned slice; internal state should be unaffected
+ first[0] = ^first[0]
+ second := detail.Bytes()
+ assert.NotEqual(t, first, second)
+}
+
+// TestWrapIfContextError verifies that wrapIfContextError wraps
+// context.Canceled and context.DeadlineExceeded with the corresponding
+// Triple codes, leaves already-coded and plain errors unchanged, and
+// returns nil for nil input.
+func TestWrapIfContextError(t *testing.T) {
+ t.Parallel()
+ // context.Canceled -> CodeCanceled
+ err := wrapIfContextError(context.Canceled)
+ tripleErr, ok := asError(err)
+ assert.True(t, ok)
+ assert.Equal(t, tripleErr.Code(), CodeCanceled)
+ assert.ErrorIs(t, err, context.Canceled)
+ // context.DeadlineExceeded -> CodeDeadlineExceeded
+ err = wrapIfContextError(context.DeadlineExceeded)
+ tripleErr, ok = asError(err)
+ assert.True(t, ok)
+ assert.Equal(t, tripleErr.Code(), CodeDeadlineExceeded)
+ assert.ErrorIs(t, err, context.DeadlineExceeded)
+ // Already coded error is returned unchanged (same instance)
+ coded := NewError(CodeNotFound, errors.New("not found"))
+ assert.True(t, wrapIfContextError(coded) == coded)
+ // Plain error is returned as-is (not coded)
+ plainErr := errors.New("plain")
+ assert.True(t, wrapIfContextError(plainErr) == plainErr)
+ // nil
+ assert.Nil(t, wrapIfContextError(nil))
+}
+
+// TestWrapIfUncoded verifies that wrapIfUncoded returns nil for nil,
+// preserves already-coded errors, wraps context.Canceled via
+// wrapIfContextError, and wraps plain errors with CodeUnknown.
+func TestWrapIfUncoded(t *testing.T) {
+ t.Parallel()
+ // nil returns nil
+ assert.Nil(t, wrapIfUncoded(nil))
+ // Already coded error is returned unchanged (same instance)
+ coded := NewError(CodeNotFound, errors.New("not found"))
+ err := wrapIfUncoded(coded)
+ assert.True(t, err == coded)
+ // context.Canceled gets wrapped with CodeCanceled
+ err = wrapIfUncoded(context.Canceled)
+ result, ok := asError(err)
+ assert.True(t, ok)
+ assert.Equal(t, result.Code(), CodeCanceled)
+ // Plain error gets wrapped with CodeUnknown
+ err = wrapIfUncoded(errors.New("plain"))
+ result, ok = asError(err)
+ assert.True(t, ok)
+ assert.Equal(t, result.Code(), CodeUnknown)
+}
diff --git a/protocol/triple/triple_protocol/header.go
b/protocol/triple/triple_protocol/header.go
index 76ac92505..f367364e7 100644
--- a/protocol/triple/triple_protocol/header.go
+++ b/protocol/triple/triple_protocol/header.go
@@ -194,6 +194,13 @@ func AppendToOutgoingContext(ctx context.Context, kv
...string) context.Context
return ctx
}
+// ExtractFromOutgoingContext returns the outgoing headers set on ctx by
+// [NewOutgoingContext] or [AppendToOutgoingContext]. It returns nil if no
+// outgoing headers have been set.
+//
+// The framework calls this internally: on the client side to populate
+// request headers before sending, and on the server side to merge
+// handler-set headers into response trailers.
func ExtractFromOutgoingContext(ctx context.Context) http.Header {
extraData, ok := ctx.Value(extraDataKey{}).(map[string]http.Header)
if !ok {
@@ -234,6 +241,8 @@ func FromIncomingContext(ctx context.Context) (http.Header,
bool) {
// SetHeader appends response headers from a server handler. The headers are
// buffered and sent with the response instead of being sent immediately.
+// It returns a [CodeInternal] error if called outside a Triple handler
+// context.
//
// For example:
//
@@ -256,6 +265,8 @@ func SetHeader(ctx context.Context, header http.Header)
error {
}
// SetTrailer appends response trailers from a server handler.
+// It returns a [CodeInternal] error if called outside a Triple handler
+// context.
//
// For example:
//
@@ -279,7 +290,8 @@ func SetTrailer(ctx context.Context, trailer http.Header)
error {
// SendHeader appends response headers from a server handler and sends them
// immediately. This is useful for streaming handlers that need to flush
headers
-// before the first message.
+// before the first message. It returns a [CodeInternal] error if called
+// outside a Triple handler context.
//
// For example:
//
diff --git a/protocol/triple/triple_protocol/header_test.go
b/protocol/triple/triple_protocol/header_test.go
index 08f6dc085..c6653d8a1 100644
--- a/protocol/triple/triple_protocol/header_test.go
+++ b/protocol/triple/triple_protocol/header_test.go
@@ -98,3 +98,136 @@ func ExampleNewOutgoingContext() {
// [triple dubbo]
// hessian
}
+
+// TestDecodeBinaryHeader verifies that DecodeBinaryHeader handles empty,
+// unpadded, padded and naturally-aligned base64 inputs.
+func TestDecodeBinaryHeader(t *testing.T) {
+ t.Parallel()
+ // Empty input
+ got, err := DecodeBinaryHeader("")
+ assert.Nil(t, err)
+ assert.Equal(t, got, []byte{})
+ // Unpadded (len % 4 != 0)
+ decoded, err := DecodeBinaryHeader(EncodeBinaryHeader([]byte("hello")))
+ assert.Nil(t, err)
+ assert.Equal(t, decoded, []byte("hello"))
+ // Padded (len % 4 == 0, has '=' padding)
+ decoded, err = DecodeBinaryHeader("aGVsbG8=")
+ assert.Nil(t, err)
+ assert.Equal(t, decoded, []byte("hello"))
+ // Naturally aligned (len % 4 == 0, no padding needed)
+ decoded, err = DecodeBinaryHeader("YWJj")
+ assert.Nil(t, err)
+ assert.Equal(t, decoded, []byte("abc"))
+}
+
+// TestExtractFromOutgoingContext verifies that ExtractFromOutgoingContext
+// returns nil when no outgoing headers are set and returns the headers
+// when they have been set via NewOutgoingContext.
+func TestExtractFromOutgoingContext(t *testing.T) {
+ t.Parallel()
+ // No outgoing headers set
+ assert.Nil(t, ExtractFromOutgoingContext(context.Background()))
+ // Headers set via NewOutgoingContext
+ ctx := NewOutgoingContext(context.Background(), http.Header{
+ "Foo": []string{"bar"},
+ })
+ extracted := ExtractFromOutgoingContext(ctx)
+ assert.NotNil(t, extracted)
+ assert.Equal(t, extracted.Get("Foo"), "bar")
+}
+
+// TestNewOutgoingContextReplacesExisting verifies that a second call to
+// NewOutgoingContext replaces the existing outgoing headers instead of
+// merging them.
+func TestNewOutgoingContextReplacesExisting(t *testing.T) {
+ t.Parallel()
+ ctx := NewOutgoingContext(context.Background(), http.Header{
+ "Foo": []string{"bar"},
+ })
+ ctx = NewOutgoingContext(ctx, http.Header{
+ "Baz": []string{"qux"},
+ })
+ extracted := ExtractFromOutgoingContext(ctx)
+ assert.Equal(t, extracted.Get("Foo"), "")
+ assert.Equal(t, extracted.Get("Baz"), "qux")
+}
+
+// TestAppendToOutgoingContextPanicsOnOddKV verifies that
+// AppendToOutgoingContext panics when given an odd number of key-value
+// arguments.
+func TestAppendToOutgoingContextPanicsOnOddKV(t *testing.T) {
+ t.Parallel()
+ assert.Panics(t, func() {
+ // Deliberately pass an odd number of kv arguments to trigger
the
+ // panic. staticcheck SA5012 flags this as a bug.
+ AppendToOutgoingContext(context.Background(), "foo")
//nolint:staticcheck
+ })
+}
+
+// mockHandlerConn is a minimal StreamingHandlerConn for testing SetHeader and
+// SetTrailer.
+type mockHandlerConn struct {
+ responseHdr http.Header
+ responseTrlr http.Header
+}
+
+func newMockHandlerConn() *mockHandlerConn {
+ return &mockHandlerConn{
+ responseHdr: make(http.Header),
+ responseTrlr: make(http.Header),
+ }
+}
+
+func (m *mockHandlerConn) Spec() Spec { return Spec{} }
+func (m *mockHandlerConn) Peer() Peer { return Peer{} }
+func (m *mockHandlerConn) Receive(any) error { return nil }
+func (m *mockHandlerConn) RequestHeader() http.Header { return nil }
+func (m *mockHandlerConn) ExportableHeader() http.Header { return nil }
+func (m *mockHandlerConn) Send(any) error { return nil }
+func (m *mockHandlerConn) ResponseHeader() http.Header { return
m.responseHdr }
+func (m *mockHandlerConn) ResponseTrailer() http.Header { return
m.responseTrlr }
+
+// TestSetHeader verifies that SetHeader merges headers into the response
+// header buffer when called within a handler context.
+func TestSetHeader(t *testing.T) {
+ t.Parallel()
+ conn := newMockHandlerConn()
+ ctx := context.WithValue(context.Background(), handlerOutgoingKey{},
conn)
+ err := SetHeader(ctx, http.Header{"X-Custom": []string{"value"}})
+ assert.Nil(t, err)
+ assert.Equal(t, conn.responseHdr.Get("X-Custom"), "value")
+}
+
+// TestSetTrailer verifies that SetTrailer merges headers into the response
+// trailer buffer when called within a handler context.
+func TestSetTrailer(t *testing.T) {
+ t.Parallel()
+ conn := newMockHandlerConn()
+ ctx := context.WithValue(context.Background(), handlerOutgoingKey{},
conn)
+ err := SetTrailer(ctx, http.Header{"X-Trailer": []string{"end"}})
+ assert.Nil(t, err)
+ assert.Equal(t, conn.responseTrlr.Get("X-Trailer"), "end")
+}
+
+// TestSetHeaderOutsideHandler verifies that SetHeader returns a CodeInternal
+// error when called outside a Triple handler context.
+func TestSetHeaderOutsideHandler(t *testing.T) {
+ t.Parallel()
+ err := SetHeader(context.Background(), http.Header{"X-Custom":
[]string{"value"}})
+ assert.NotNil(t, err)
+ tripleErr, ok := asError(err)
+ assert.True(t, ok)
+ assert.Equal(t, tripleErr.Code(), CodeInternal)
+}
+
+// TestSetTrailerOutsideHandler verifies that SetTrailer returns a
+// CodeInternal error when called outside a Triple handler context.
+func TestSetTrailerOutsideHandler(t *testing.T) {
+ t.Parallel()
+ err := SetTrailer(context.Background(), http.Header{"X-Trailer":
[]string{"end"}})
+ assert.NotNil(t, err)
+ tripleErr, ok := asError(err)
+ assert.True(t, ok)
+ assert.Equal(t, tripleErr.Code(), CodeInternal)
+}
