(powersensor/sensor) Guard against nil responses in rdk client/servers (viamrobotics#4621)

Author: martha-johnston

components/powersensor/client_test.go

@@ -11,6 +11,7 @@ import (
 
 	"go.viam.com/rdk/components/motor"
 	"go.viam.com/rdk/components/powersensor"
+	"go.viam.com/rdk/components/sensor"
 	viamgrpc "go.viam.com/rdk/grpc"
 	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/resource"
@@ -57,6 +58,10 @@ func TestClient(t *testing.T) {
 		return 0, errPowerFailed
 	}
 
+	failingPowerSensor.ReadingsFunc = func(ctx context.Context, extra map[string]interface{}) (map[string]interface{}, error) {
+		return nil, nil
+	}
+
 	resourceMap := map[resource.Name]powersensor.PowerSensor{
 		motor.Named(workingPowerSensorName): workingPowerSensor,
 		motor.Named(failingPowerSensorName): failingPowerSensor,
@@ -134,6 +139,9 @@ func TestClient(t *testing.T) {
 		test.That(t, err.Error(), test.ShouldContainSubstring, errPowerFailed.Error())
 		test.That(t, watts, test.ShouldEqual, 0)
 
+		_, err = client.Readings(context.Background(), make(map[string]interface{}))
+		test.That(t, err.Error(), test.ShouldContainSubstring, sensor.ErrReadingsNil("power-sensor", failingPowerSensorName).Error())
+
 		test.That(t, client.Close(context.Background()), test.ShouldBeNil)
 		test.That(t, conn.Close(), test.ShouldBeNil)
 	})

components/powersensor/server.go

@@ -6,6 +6,7 @@ import (
 	commonpb "go.viam.com/api/common/v1"
 	pb "go.viam.com/api/component/powersensor/v1"
 
+	"go.viam.com/rdk/components/sensor"
 	"go.viam.com/rdk/protoutils"
 	"go.viam.com/rdk/resource"
 )
@@ -33,6 +34,9 @@ func (s *serviceServer) GetReadings(
 	if err != nil {
 		return nil, err
 	}
+	if readings == nil {
+		return nil, sensor.ErrReadingsNil("power-sensor", req.Name)
+	}
 	m, err := protoutils.ReadingGoToProto(readings)
 	if err != nil {
 		return nil, err

components/powersensor/server_test.go

@@ -179,6 +179,14 @@ func TestServerGetReadings(t *testing.T) {
 	test.That(t, err, test.ShouldNotBeNil)
 	test.That(t, err.Error(), test.ShouldContainSubstring, errReadingsFailed.Error())
 
+	failingPowerSensor.ReadingsFunc = func(ctx context.Context, extra map[string]interface{}) (map[string]interface{}, error) {
+		return nil, nil
+	}
+
+	_, err = powerSensorServer.GetReadings(context.Background(), &commonpb.GetReadingsRequest{Name: failingPowerSensorName})
+	test.That(t, err, test.ShouldNotBeNil)
+	test.That(t, err.Error(), test.ShouldContainSubstring, sensor.ErrReadingsNil("power-sensor", failingPowerSensorName).Error())
+
 	_, err = powerSensorServer.GetReadings(context.Background(), &commonpb.GetReadingsRequest{Name: missingPowerSensorName})
 	test.That(t, err, test.ShouldNotBeNil)
 	test.That(t, err.Error(), test.ShouldContainSubstring, "not found")

components/sensor/client_test.go

@@ -106,6 +106,13 @@ func TestClient(t *testing.T) {
 		test.That(t, err, test.ShouldNotBeNil)
 		test.That(t, err.Error(), test.ShouldContainSubstring, errReadingsFailed.Error())
 
+		injectSensor2.ReadingsFunc = func(ctx context.Context, extra map[string]interface{}) (map[string]interface{}, error) {
+			return nil, nil
+		}
+		_, err = client2.Readings(context.Background(), make(map[string]interface{}))
+		test.That(t, err, test.ShouldNotBeNil)
+		test.That(t, err.Error(), test.ShouldContainSubstring, sensor.ErrReadingsNil("sensor", failSensorName).Error())
+
 		test.That(t, client2.Close(context.Background()), test.ShouldBeNil)
 		test.That(t, conn.Close(), test.ShouldBeNil)
 	})

components/sensor/server.go

@@ -3,6 +3,7 @@ package sensor
 
 import (
 	"context"
+	"fmt"
 
 	commonpb "go.viam.com/api/common/v1"
 	pb "go.viam.com/api/component/sensor/v1"
@@ -11,6 +12,11 @@ import (
 	"go.viam.com/rdk/resource"
 )
 
+// ErrReadingsNil is the returned error if sensor readings are nil.
+var ErrReadingsNil = func(sensorType, sensorName string) error {
+	return fmt.Errorf("%v component %v Readings should not return nil readings", sensorType, sensorName)
+}
+
 // serviceServer implements the SensorService from sensor.proto.
 type serviceServer struct {
 	pb.UnimplementedSensorServiceServer
@@ -35,6 +41,9 @@ func (s *serviceServer) GetReadings(
 	if err != nil {
 		return nil, err
 	}
+	if readings == nil {
+		return nil, ErrReadingsNil("sensor", req.Name)
+	}
 	m, err := protoutils.ReadingGoToProto(readings)
 	if err != nil {
 		return nil, err

components/sensor/server_test.go

@@ -67,6 +67,13 @@ func TestServer(t *testing.T) {
 		test.That(t, err, test.ShouldNotBeNil)
 		test.That(t, err.Error(), test.ShouldContainSubstring, errReadingsFailed.Error())
 
+		injectSensor2.ReadingsFunc = func(ctx context.Context, extra map[string]interface{}) (map[string]interface{}, error) {
+			return nil, nil
+		}
+		_, err = sensorServer.GetReadings(context.Background(), &commonpb.GetReadingsRequest{Name: failSensorName})
+		test.That(t, err, test.ShouldNotBeNil)
+		test.That(t, err.Error(), test.ShouldContainSubstring, sensor.ErrReadingsNil("sensor", failSensorName).Error())
+
 		_, err = sensorServer.GetReadings(context.Background(), &commonpb.GetReadingsRequest{Name: missingSensorName})
 		test.That(t, err, test.ShouldNotBeNil)
 		test.That(t, err.Error(), test.ShouldContainSubstring, "not found")

protoutils/sensor.go

@@ -125,6 +125,9 @@ func ReadingGoToProto(readings map[string]interface{}) (map[string]*structpb.Val
 
 // ReadingProtoToGo converts proto readings to go readings.
 func ReadingProtoToGo(readings map[string]*structpb.Value) (map[string]interface{}, error) {
+	if readings == nil {
+		return nil, nil
+	}
 	m := map[string]interface{}{}
 	for k, v := range readings {
 		m[k] = cleanSensorType(v.AsInterface())