Alanxtl commented on code in PR #3474:
URL: https://github.com/apache/dubbo-go/pull/3474#discussion_r3534155792
##########
remoting/getty/getty_server.go:
##########
@@ -97,21 +97,35 @@ func initServer(url *common.URL) {
return
}
- gettyServerConfigBytes, err := yaml.Marshal(gettyServerConfig)
+ gettyServerConfigBytes, err :=
safeYAMLMarshal(gettyServerConfig)
if err != nil {
- panic(err)
+ logger.Errorf("[Remoting][Getty] failed to marshal
getty server config, err=%v", err)
+ return
}
err = yaml.Unmarshal(gettyServerConfigBytes, srvConf)
if err != nil {
- panic(err)
+ logger.Errorf("[Remoting][Getty] failed to unmarshal
getty server config, err=%v", err)
+ return
}
}
if err := srvConf.CheckValidity(); err != nil {
- panic(err)
+ logger.Errorf("[Remoting][Getty] server config is invalid,
err=%v", err)
+ return
}
}
+// safeYAMLMarshal wraps yaml.Marshal with a recover so that types unsupported
by
+// the yaml encoder (e.g. channels, funcs) return an error instead of
panicking.
+func safeYAMLMarshal(v any) (out []byte, err error) {
+ defer func() {
+ if r := recover(); r != nil {
+ err = fmt.Errorf("yaml marshal panic: %v", r)
+ }
+ }()
+ return yaml.Marshal(v)
+}
Review Comment:
what's the necessity of introduce a `safeYAMLMarshal`?
it's over designing
--
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]
