-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Startup related fixes #1126
base: main
Are you sure you want to change the base?
Startup related fixes #1126
Changes from all commits
e4d1c9c
0f03f60
d28d6fe
086dfc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -413,6 +413,12 @@ | |
return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, | ||
handler grpc.UnaryHandler) (interface{}, error) { | ||
|
||
if info.Server == nil { | ||
return nil, fmt.Errorf("server is nil, can't check " + | ||
"RPC state") | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: linter complains about superfluous empty line. |
||
} | ||
|
||
if err := r.checkRPCState(info.Server); err != nil { | ||
return nil, err | ||
} | ||
|
@@ -427,6 +433,10 @@ | |
return func(srv interface{}, ss grpc.ServerStream, | ||
info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { | ||
|
||
if srv == nil { | ||
return fmt.Errorf("srv is nil, can't check RPC state") | ||
} | ||
|
||
if err := r.checkRPCState(srv); err != nil { | ||
return err | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -702,11 +702,16 @@ func (s *Server) waitForReady() error { | |
// this part of the code, the called component will handle the quit | ||
// signal. | ||
select { | ||
case <-s.ready: | ||
return nil | ||
|
||
case <-s.quit: | ||
return fmt.Errorf("tapd is shutting down") | ||
|
||
default: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice fix. This probably explains one class of potential errors we had. |
||
select { | ||
case <-s.ready: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe mention here that we now wait until the server becomes ready or shuts down (meaning this is blocking). |
||
return nil | ||
case <-s.quit: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this doesn't solve the |
||
return fmt.Errorf("tapd is shutting down") | ||
} | ||
} | ||
} | ||
|
||
|
@@ -819,17 +824,25 @@ func (s *Server) Name() protofsm.EndpointName { | |
// CanHandle returns true if the target message can be routed to this endpoint. | ||
// | ||
// NOTE: This method is part of the protofsm.MsgEndpoint interface. | ||
func (s *Server) CanHandle(msg protofsm.PeerMsg) bool { | ||
<-s.ready | ||
func (s *Server) CanHandle(msg protofsm.PeerMsg) (bool, error) { | ||
err := s.waitForReady() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Depending on the discussion on the |
||
if err != nil { | ||
return false, err | ||
} | ||
|
||
return s.cfg.AuxFundingController.CanHandle(msg) | ||
} | ||
|
||
// SendMessage handles the target message, and returns true if the message was | ||
// able to be processed. | ||
// | ||
// NOTE: This method is part of the protofsm.MsgEndpoint interface. | ||
func (s *Server) SendMessage(msg protofsm.PeerMsg) bool { | ||
<-s.ready | ||
func (s *Server) SendMessage(msg protofsm.PeerMsg) (bool, error) { | ||
err := s.waitForReady() | ||
if err != nil { | ||
return false, err | ||
} | ||
|
||
return s.cfg.AuxFundingController.SendMessage(msg) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can probably be a bit less specific in the error message here, perhaps just "cannot handle call, server not ready"?