[GitHub] [beam] lostluck commented on a change in pull request #12170: [BEAM-10166] Improve execution time errors with more concise error messages from DoFns

[GitBox <gi...@apache.org>]

lostluck commented on a change in pull request #12170:
URL: https://github.com/apache/beam/pull/12170#discussion_r450470297



##########
File path: sdks/go/pkg/beam/core/runtime/exec/util.go
##########
@@ -27,6 +28,28 @@ type GenID struct {
 	last int
 }
 
+// doFnError is a helpful error where DoFns with emitters have failed.
+//
+// Golang SDK uses errors returned by DoFns to signal failures to process
+// bundles and terminate bundle processing. However, in the case of  emitters,
+// a panic is used as a means of bundle process termination, which  in turn is
+// wrapped by callNoPanic to recover the panicking code and return golang error
+// instead. However, formatting this error yields the stack trace a message
+// which is both long, and unhelpful the user.
+//
+// This error enables to create a more helpful user error where a panic is
+// caught where the origin is due to a failed DoFn execution in process bundle
+// execution.
+type doFnError struct {
+	doFn string
+	err  error
+	uid  UnitID
+	pid  string
+}
+func (e *doFnError) Error() string {
+	return fmt.Sprintf("%v caused error %v, uid is %v pid is %v", e.doFn, e.err, e.uid, e.pid)

Review comment:
       The wiki syntax in JIRA really butchered my intended suggestion of `DoFn[<uid>;<pid>]<doFn> returned error:<error>`
   
   Which places the  short/known length things before the potentially long and rife with newlines error. It puts the context first, and avoids it getting lost or truncated behind a very long error string.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org