Skip to content

GRPCClient Ping method does not check health response #341

@mortenson

Description

@mortenson

I've been working on a Node based plugin and was testing to see if my health service is working, and I noticed that the current GRPCClient implementation for Ping() doesn't actually evaluate if the service is healthy (SERVING):

go-plugin/grpc_client.go

Lines 127 to 134 in cfdf485

func (c *GRPCClient) Ping() error {
client := grpc_health_v1.NewHealthClient(c.Conn)
_, err := client.Check(context.Background(), &grpc_health_v1.HealthCheckRequest{
Service: GRPCServiceName,
})
return err
}

Instead of just checking for err (which would tell you if the service was "hard down", or didn't implement /grpc.health.v1.Health/Check), this should also check the response from client.Check to check that it returned grpc_health_v1.HealthCheckResponse_SERVING.

Changing this would probably break a lot of people's stuff right now, but I thought I'd point it out.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions