diff --git a/controller/leaderboard_controller.go b/controller/leaderboard_controller.go index bb1a4626..7e0b9039 100644 --- a/controller/leaderboard_controller.go +++ b/controller/leaderboard_controller.go @@ -23,6 +23,12 @@ func GetLeaderboard( earners, err := model.GetLeaderboard(session.Ctx) if err != nil { return &model.LeaderboardResult{ + // Earners must be non-nil even on the error path: the client + // deserialises the whole result before reading Error, and a nil + // slice here marshals as `null`, which crashes the android client + // exactly as an empty leaderboard did. A transient query error must + // not take the app down. + Earners: []model.Earner{}, Error: &model.TopEarnersError{ Message: err.Error(), }, diff --git a/model/account_point_model.go b/model/account_point_model.go index 410003f5..75518246 100644 --- a/model/account_point_model.go +++ b/model/account_point_model.go @@ -131,6 +131,11 @@ func ApplyAccountPointsBatchInTx( } func FetchAccountPoints(ctx context.Context, networkId server.Id) (accountPoints []AccountPoint) { + // non-nil for the same reason as GetLeaderboard: a nil slice becomes JSON + // `null`, which the gomobile binding turns into a nil pointer and the + // android client crashes on. Feeds both `network_points` and its + // `account_points` alias. + accountPoints = []AccountPoint{} server.Db(ctx, func(conn server.PgConn) { result, err := conn.Query( diff --git a/model/leaderboard_model.go b/model/leaderboard_model.go index d9e152eb..27cb0231 100644 --- a/model/leaderboard_model.go +++ b/model/leaderboard_model.go @@ -28,6 +28,11 @@ type TopEarnersError struct { * Gets an ordered list of the top earners */ func GetLeaderboard(ctx context.Context) (earners []Earner, queryErr error) { + // must be non-nil: a nil slice marshals as JSON `null`, and the gomobile + // sdk binds this field as a pointer, so `null` becomes a nil object that + // the android client dereferences inside a jni callback -- the NPE cannot + // cross jni and ART aborts the whole process. An empty leaderboard is `[]`. + earners = []Earner{} // stats read: tolerates replica delay server.ReplicaDb(ctx, func(conn server.PgConn) { diff --git a/model/nil_slice_json_test.go b/model/nil_slice_json_test.go new file mode 100644 index 00000000..63825df9 --- /dev/null +++ b/model/nil_slice_json_test.go @@ -0,0 +1,63 @@ +package model + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/urnetwork/server" +) + +// A nil slice marshals as JSON `null`. The gomobile sdk binds these fields as +// pointers, so `null` becomes a nil object the android client dereferences +// inside a jni callback; the NPE cannot cross jni and ART aborts the process. +// An API slice that can legitimately be empty must serialise as `[]`. +// +// These assert on the marshalled bytes, because `[]` versus `null` is the +// property that actually reaches the client. + +func TestGetLeaderboardEmptyMarshalsAsArrayNotNull(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + // a fresh db has no payouts, so the query returns zero rows -- the + // exact condition that crashed the android client + earners, err := GetLeaderboard(ctx) + if err != nil { + t.Fatalf("GetLeaderboard: %s", err) + } + if earners == nil { + t.Fatal("GetLeaderboard returned a nil slice; it marshals as null and crashes the client") + } + + b, err := json.Marshal(LeaderboardResult{Earners: earners}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `"earners":[]`) { + t.Errorf("marshalled as %s, want \"earners\":[]", b) + } + }) +} + +func TestFetchAccountPointsEmptyMarshalsAsArrayNotNull(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + // a network with no points at all -- feeds both `network_points` and + // its `account_points` alias + points := FetchAccountPoints(ctx, server.NewId()) + if points == nil { + t.Fatal("FetchAccountPoints returned a nil slice; it marshals as null and crashes the client") + } + + b, err := json.Marshal(map[string]any{"network_points": points}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), "null") { + t.Errorf("marshalled as %s, want an empty array", b) + } + }) +}