HarshMehta112 commented on code in PR #3474:
URL: https://github.com/apache/dubbo-go/pull/3474#discussion_r3542143417
##########
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:
@Alanxtl Closing as **Not planned**.
`yaml.Marshal` can panic on cyclic data, and `Params` is `any`, so the
wrapper intentionally returns an error instead of letting the server crash.
Unless you can guarantee `Params` is never cyclic, there's nothing more to
discuss here.
--
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]
