[beam] branch master updated: [BEAM-7393] Misc. improvements to Go error formatting.

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

altay pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git


The following commit(s) were added to refs/heads/master by this push:
     new 6d75846  [BEAM-7393] Misc. improvements to Go error formatting.
     new 4e20419  Merge pull request #8655 from youngoli/go-errors
6d75846 is described below

commit 6d758469d1ae5ae80c62cde5783b3da8bdf1aa9b
Author: Daniel Oliveira <da...@gmail.com>
AuthorDate: Wed May 22 16:14:40 2019 -0700

    [BEAM-7393] Misc. improvements to Go error formatting.
    
    Mainly two changes:
    1. Remove default top-level error messages. If a top-level msg wasn't
       set then just don't display any.
    2. Align the "Caused by:" msg on wrapped errors with the context
       messages, to improve readability.
---
 sdks/go/pkg/beam/core/runtime/graphx/coder.go   |  5 ++---
 sdks/go/pkg/beam/internal/errors/errors.go      | 17 ++++++++---------
 sdks/go/pkg/beam/internal/errors/errors_test.go |  4 ++--
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/sdks/go/pkg/beam/core/runtime/graphx/coder.go b/sdks/go/pkg/beam/core/runtime/graphx/coder.go
index 50e4591..f37eb01 100644
--- a/sdks/go/pkg/beam/core/runtime/graphx/coder.go
+++ b/sdks/go/pkg/beam/core/runtime/graphx/coder.go
@@ -321,10 +321,9 @@ func (b *CoderMarshaller) Add(c *coder.Coder) string {
 		ref, err := encodeCustomCoder(c.Custom)
 		if err != nil {
 			typeName := c.Custom.Name
-			panic(fmt.Sprintf("Failed to encode custom coder for type %s. "+
+			panic(errors.SetTopLevelMsgf(err, "Failed to encode custom coder for type %s. "+
 				"Make sure the type was registered before calling beam.Init. For example: "+
-				"beam.RegisterType(reflect.TypeOf((*TypeName)(nil)).Elem())\n\n"+
-				"Full error: %v", typeName, err))
+				"beam.RegisterType(reflect.TypeOf((*TypeName)(nil)).Elem())", typeName))
 		}
 		data, err := protox.EncodeBase64(ref)
 		if err != nil {
diff --git a/sdks/go/pkg/beam/internal/errors/errors.go b/sdks/go/pkg/beam/internal/errors/errors.go
index cd3269c..dde5d81 100644
--- a/sdks/go/pkg/beam/internal/errors/errors.go
+++ b/sdks/go/pkg/beam/internal/errors/errors.go
@@ -115,7 +115,7 @@ func getTop(e error) string {
 	if be, ok := e.(*beamError); ok {
 		return be.top
 	}
-	return e.Error()
+	return ""
 }
 
 // beamError represents one or more details about an error. They are usually
@@ -127,10 +127,10 @@ func getTop(e error) string {
 //
 // * If no cause is present it indicates that this instance is the original
 //   error, and the message is assumed to be present.
-// * If both message and context are present, the context describes this error
-//   not the next error.
-// * top is always assumed to be present since it is propogated up from the
-//   original error if not explicitly set.
+// * If both message and context are present, the context describes this error,
+//   not the cause of this error.
+// * top is always propogated up from the cause. If it's empty that means that
+//   it was never set on any error in the sequence.
 type beamError struct {
 	cause   error  // The error being wrapped. If nil then this is the first error.
 	context string // Adds additional context to this error and any following.
@@ -153,9 +153,8 @@ func (e *beamError) Error() string {
 	return builder.String()
 }
 
-// printRecursive outputs the contexts and messages of beamErrors recursively
-// while ignoring the top-level error. This avoids calling Error recursively on
-// beamErrors since that would repeatedly print top-level messages.
+// printRecursive is a helper function for outputting the contexts and messages
+// of a sequence of beamErrors.
 func (e *beamError) printRecursive(builder *strings.Builder) {
 	wraps := e.cause != nil
 
@@ -165,7 +164,7 @@ func (e *beamError) printRecursive(builder *strings.Builder) {
 	if e.msg != "" {
 		builder.WriteString(e.msg)
 		if wraps {
-			builder.WriteString("\nCaused by:\n")
+			builder.WriteString("\n\tcaused by:\n")
 		}
 	}
 
diff --git a/sdks/go/pkg/beam/internal/errors/errors_test.go b/sdks/go/pkg/beam/internal/errors/errors_test.go
index 5df0da0..614609f 100644
--- a/sdks/go/pkg/beam/internal/errors/errors_test.go
+++ b/sdks/go/pkg/beam/internal/errors/errors_test.go
@@ -116,10 +116,10 @@ func TestTopLevelMsg(t *testing.T) {
 	}{
 		{
 			err:  New(base),
-			want: base,
+			want: "",
 		}, {
 			err:  Wrap(WithContext(New(base), ctx1), msg1),
-			want: base,
+			want: "",
 		}, {
 			err:  SetTopLevelMsg(New(base), top1),
 			want: top1,