Skip to content

Commit

Permalink
[APP-3853] Remove non-ASCII characters from sensor-reading errors (vi…
Browse files Browse the repository at this point in the history
…amrobotics#3769)

See comment in the code for the explanation, and see the ticket for how we tracked that down.
  • Loading branch information
penguinland authored Apr 4, 2024
1 parent cdf2229 commit fb90a8b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
16 changes: 15 additions & 1 deletion protoutils/sensor.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package protoutils

import (
"errors"
"strings"

"github.com/golang/geo/r3"
geo "github.com/kellydunn/golang-geo"
"google.golang.org/protobuf/types/known/structpb"
Expand Down Expand Up @@ -91,7 +94,18 @@ func goToProto(v interface{}) (*structpb.Value, error) {
}
}

return structpb.NewValue(v)
vv, err := structpb.NewValue(v)
// The structpb package sometimes inserts non-breaking spaces into their error messages (see
// https://github.com/protocolbuffers/protobuf-go/blob/master/internal/errors/errors.go#L30).
// However, we put error messages into the headers/trailers of GRPC responses, and client-side
// parsing of those requires that they must be entirely ASCII. So, we need to ensure that any
// errors from here do not contain non-breaking spaces.
if err != nil {
ascii := strings.ReplaceAll(
err.Error(), " " /* non-breaking space */, " " /* normal space */)
err = errors.New(ascii)
}
return vv, err
}

// ReadingGoToProto converts go readings to proto readings.
Expand Down
11 changes: 11 additions & 0 deletions protoutils/sensor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package protoutils

import (
"testing"
"time"

"github.com/golang/geo/r3"
geo "github.com/kellydunn/golang-geo"
Expand Down Expand Up @@ -31,3 +32,13 @@ func TestRoundtrip(t *testing.T) {

test.That(t, m2, test.ShouldResemble, m1)
}

func TestInvalidValue(t *testing.T) {
data := map[string]interface{}{
"now": time.Now(),
}

_, err := ReadingGoToProto(data)
test.That(t, err, test.ShouldNotBeNil)
test.That(t, err.Error(), test.ShouldEqual, "proto: invalid type: time.Time")
}

0 comments on commit fb90a8b

Please sign in to comment.