Datastore Count panics with unhelpful message if query contains invalid type

[haleyrc]

Client

Datastore

Describe Your Environment

Alpine Docker on Linux

Expected Behavior

When Count is run with a query containing an invalid type, an error is returned indicating a bad type in the query. For example, given the following:

type MyType int
var v MyType = 1
q := datastore.NewQuery("MyKind").Filter("fld=", v)
n, err := c.Count(ctx, q)

err should be similar to datastore: bad query filter value type: invalid Value type MyType so that the filter value can correctly be cast to an int.

This should be trivially accomplished by adding a check for an error on the iterator in Count.

Actual Behavior

The program crashes when it.nextBatch() is called in Count as t.req.GetQuery() returns nil.

[jeanbza]

Could you paste the error that you're seeing?

[haleyrc]

Absolutely

2019/02/06 19:05:15 http: panic serving 172.22.0.1:55482: runtime error: invalid memory address or nil pointer dereference
goroutine 51 [running]:
net/http.(*conn).serve.func1(0xc000324aa0)
 /usr/local/go/src/net/http/server.go:1746 +0xd0
panic(0xa12ae0, 0x104d3c0)
 /usr/local/go/src/runtime/panic.go:513 +0x1b9
cloud.google.com/go/datastore.(*Iterator).nextBatch(0xc00046c480, 0xbdf800, 0xc000320f90)
 /go/src/cloud.google.com/go/datastore/query.go:683 +0x6a
cloud.google.com/go/datastore.(*Client).Count(0xc0004c2540, 0xbdf800, 0xc000320f90, 0xc0004ce870, 0x0, 0x0, 0x0)
 /go/src/cloud.google.com/go/datastore/query.go:471 +0x1a3
github.com/frazercomputing/cloud-services/backend/pkg/repo/datastore.(*employeeRepository).CountEnabledAdmins(0xc00055e2a0, 0xbdf800, 0xc000149860, 0xc0003000d0, 0x5, 0xc000015448, 0xbf0ee9daf2ee844c, 0x58fa1248)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/repo/datastore/employee.go:333 +0x1b8
github.com/frazercomputing/cloud-services/backend/pkg/repo.(*loggingEmployeeRepository).CountEnabledAdmins(0xc000562340, 0xbdf800, 0xc000149860, 0xc0003000d0, 0x5, 0x0, 0x0, 0x0)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/repo/employee.go:66 +0xe0
github.com/frazercomputing/cloud-services/backend/pkg/service/dealership.dealershipService.CountEnabledAdmins(0xbdc2c0, 0xc000562320, 0xbe2420, 0xc000562340,0xbd9340, 0xc000562360, 0xbd9380, 0xc0005623a0, 0xbdf9c0, 0xc0005623c0, ...)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/service/dealership/dealership.go:164 +0x69
github.com/frazercomputing/cloud-services/backend/pkg/service/dealership.dealershipService.DisableEmployee(0xbdc2c0, 0xc000562320, 0xbe2420, 0xc000562340, 0xbd9340, 0xc000562360, 0xbd9380, 0xc0005623a0, 0xbdf9c0, 0xc0005623c0, ...)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/service/dealership/dealership.go:173 +0x28c
github.com/frazercomputing/cloud-services/backend/pkg/service/dealership.(*authorizedService).DisableEmployee(0xc000562480, 0xbdf800, 0xc000149860, 0xc0003000d0, 0x5, 0xc00030e520, 0x1f, 0x0, 0x0)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/service/dealership/dealership.go:423 +0xe1
github.com/frazercomputing/cloud-services/backend/pkg/service/dealership.(*analyticsService).DisableEmployee(0xc0005624a0, 0xbdf800, 0xc000149860, 0xc0003000d0, 0x5, 0xc00030e520, 0x1f, 0xc0004ff7d8, 0xbf0ee9daf1e72478)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/service/dealership/analytics.go:33 +0xf8
github.com/frazercomputing/cloud-services/backend/pkg/service/dealership.(*loggingService).DisableEmployee(0xc0005624c0, 0xbdf800, 0xc000149860, 0xc0003000d0, 0x5, 0xc00030e520, 0x1f, 0x0, 0x0)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/service/dealership/logging.go:65 +0x118
github.com/frazercomputing/cloud-services/backend/pkg/transport.(*employeeHandler).Disable(0xc000562700, 0xbdc240, 0xc0002d8120, 0xc00044a600)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/transport/employee.go:136 +0x142
github.com/frazercomputing/cloud-services/backend/pkg/transport.EmployeeHandler.Disable-fm(0xbdc240, 0xc0002d8120, 0xc00044a600)
 /go/src/github.com/frazercomputing/cloud-services/backend/cmd/f4v2/main.go:272 +0x4d
github.com/frazercomputing/cloud-services/backend/pkg/transport/auth.AuthenticateUserWithToken.func1.1(0xbdc240, 0xc0002d8120, 0xc00044a500)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/transport/auth/userauth.go:44 +0x5a6
github.com/frazercomputing/cloud-services/backend/pkg/transport.(*authenticatedEmployeeHandler).Disable(0xc000562720, 0xbdc240, 0xc0002d8120, 0xc00044a500)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/transport/employee.go:315 +0xac
github.com/frazercomputing/cloud-services/backend/pkg/transport.EmployeeHandler.Disable-fm(0xbdc240, 0xc0002d8120, 0xc00044a500)
 /go/src/github.com/frazercomputing/cloud-services/backend/cmd/f4v2/main.go:272 +0x4d
net/http.HandlerFunc.ServeHTTP(0xc0005631c0, 0xbdc240, 0xc0002d8120, 0xc00044a500)
 /usr/local/go/src/net/http/server.go:1964 +0x44
github.com/gorilla/mux.(*Router).ServeHTTP(0xc00055a2a0, 0xbdc240, 0xc0002d8120, 0xc00044a500)
 /go/src/github.com/gorilla/mux/mux.go:162 +0xf1
github.com/frazercomputing/cloud-services/backend/pkg/lib/middleware.Stats.func1.1(0xbdc240, 0xc0002d8100, 0xc00044a100)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/lib/middleware/stats.go:51 +0x3ec
net/http.HandlerFunc.ServeHTTP(0xc0004b9cb0, 0xbdc240, 0xc0002d8100, 0xc00044a100)
 /usr/local/go/src/net/http/server.go:1964 +0x44
github.com/frazercomputing/cloud-services/backend/pkg/lib/middleware.Report.func1(0xbdee80, 0xc000340000, 0xc00044a100)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/lib/middleware/error.go:13 +0xa7
net/http.HandlerFunc.ServeHTTP(0xc000563c00, 0xbdee80, 0xc000340000, 0xc00044a100)
 /usr/local/go/src/net/http/server.go:1964 +0x44
github.com/frazercomputing/cloud-services/backend/pkg/lib/middleware.CORS.func1(0xbdee80, 0xc000340000, 0xc00044a100)
 /go/src/github.com/frazercomputing/cloud-services/backend/pkg/lib/middleware/cors.go:14 +0x169
net/http.HandlerFunc.ServeHTTP(0xc000563c20, 0xbdee80, 0xc000340000, 0xc00044a100)
 /usr/local/go/src/net/http/server.go:1964 +0x44
net/http.serverHandler.ServeHTTP(0xc0001e6410, 0xbdee80, 0xc000340000, 0xc00044a100)
 /usr/local/go/src/net/http/server.go:2741 +0xab
net/http.(*conn).serve(0xc000324aa0, 0xbdf740, 0xc0002e8200)
 /usr/local/go/src/net/http/server.go:1847 +0x646
created by net/http.(*Server).Serve
 /usr/local/go/src/net/http/server.go:2851 +0x2f5

[poy]

After looking into this, I would say the problem is that Filter() is not validating the type and setting the query's error.

Instead of adding some code to protect against this in Count(), I think it would be more appropriate to move the type validation to when Filter() is initially called (instead of when the query is ran).