fix(milpacs): route errors to NotFound vs Internal

datastores/mysql.go

@@ -32,9 +32,6 @@ func (ds Mysql) FindProfilesById(userIds ...uint64) ([]*proto.Profile, error) {
 		First(&profile, userIds[0])
 
 	if result.Error != nil {
-		if result.Error == gorm.ErrRecordNotFound {
-			return nil, fmt.Errorf("no profile found for userid: %d", userIds)
-		}
 		return nil, result.Error
 	}
 
@@ -60,9 +57,6 @@ func (ds Mysql) FindProfilesByUsername(username string) ([]*proto.Profile, error
 		First(&profile)
 
 	if result.Error != nil {
-		if result.Error == gorm.ErrRecordNotFound {
-			return nil, fmt.Errorf("no profile found for username: %s", username)
-		}
 		return nil, result.Error
 	}
 
@@ -79,11 +73,14 @@ func (ds Mysql) FindRosterByType(rosterType proto.RosterType) (*proto.Roster, er
 	var rosterProfiles []milpacs.Profile
 
 	Info.Println("Searching for roster: ", rosterType.String(), "id:", uint(rosterType.Number()))
-	ds.Db.Preload(clause.Associations).
+	result := ds.Db.Preload(clause.Associations).
 		Preload("AwardRecords.Award").
 		Joins(xenforo.ConnectedAccountJoin).
 		Where(map[string]interface{}{"roster_id": uint(rosterType.Number())}).
 		Find(&rosterProfiles)
+	if result.Error != nil {
+		return nil, fmt.Errorf("find roster %s: %w", rosterType, result.Error)
+	}
 
 	profiles, err := ds.processProfiles(rosterProfiles)
 	if err != nil {
@@ -108,9 +105,6 @@ func (ds Mysql) FindProfileByKeycloakID(keycloakId string) (*proto.Profile, erro
 		First(&profile)
 
 	if result.Error != nil {
-		if result.Error == gorm.ErrRecordNotFound {
-			return nil, fmt.Errorf("no profile found for KeycloakID: %s", keycloakId)
-		}
 		return nil, result.Error
 	}
 
@@ -135,9 +129,6 @@ func (ds Mysql) FindProfileByDiscordID(discordId string) (*proto.Profile, error)
 		First(&profile)
 
 	if result.Error != nil {
-		if result.Error == gorm.ErrRecordNotFound {
-			return nil, fmt.Errorf("no profile found for discordID: %s", discordId)
-		}
 		return nil, result.Error
 	}
 
@@ -266,11 +257,14 @@ func (ds Mysql) FindLiteRosterByType(rosterType proto.RosterType) (*proto.LiteRo
 	var rosterProfiles []milpacs.Profile
 
 	Info.Println("Searching for lite roster: ", rosterType.String(), "id:", uint(rosterType.Number()))
-	ds.Db.Preload(clause.Associations).
+	result := ds.Db.Preload(clause.Associations).
 		Omit("Records", "AwardRecords").
 		Joins(xenforo.ConnectedAccountJoin).
 		Where(map[string]interface{}{"roster_id": uint(rosterType.Number())}).
 		Find(&rosterProfiles)
+	if result.Error != nil {
+		return nil, fmt.Errorf("find lite roster %s: %w", rosterType, result.Error)
+	}
 
 	profiles, err := ds.processLiteProfiles(rosterProfiles)
 	if err != nil {
@@ -347,12 +341,15 @@ func (ds Mysql) FindS1UniformsRosterByType(rosterType proto.RosterType) (*proto.
 	var rosterProfiles []milpacs.Profile
 
 	Info.Println("Searching for S1 Uniforms roster: ", rosterType.String(), "id:", uint(rosterType.Number()))
-	ds.Db.Preload(clause.Associations).
+	result := ds.Db.Preload(clause.Associations).
 		Preload("Primary.Group").
 		Preload("AwardRecords.Award").
 		Joins(xenforo.ConnectedAccountJoin).
 		Where(map[string]interface{}{"roster_id": uint(rosterType.Number())}).
 		Find(&rosterProfiles)
+	if result.Error != nil {
+		return nil, fmt.Errorf("find s1 uniforms roster %s: %w", rosterType, result.Error)
+	}
 
 	var profiles = make(map[uint64]*proto.S1UniformsProfile, len(rosterProfiles))
 	for _, profile := range rosterProfiles {
@@ -750,9 +747,6 @@ func (ds Mysql) FindProfileByGamertag(gamertag string) (*proto.Profile, error) {
 		First(&profile)
 
 	if result.Error != nil {
-		if result.Error == gorm.ErrRecordNotFound {
-			return nil, fmt.Errorf("no profile found for gamertag: %s", gamertag)
-		}
 		return nil, result.Error
 	}
 

servers/grpc/fake_datastore_test.go

@@ -0,0 +1,107 @@
+package grpc
+
+import (
+	"context"
+
+	"github.com/7cav/api/datastores"
+	"github.com/7cav/api/proto"
+	"github.com/7cav/api/xenforo"
+)
+
+// fakeDatastore is the shared in-process Datastore stub used by both
+// tickets_test.go and milpacs_test.go. Each handler-under-test gets the
+// matching function field populated; everything else either panics (methods
+// this PR does not exercise) or nil-derefs on call (unset configurable
+// fields) so an accidentally-untouched test fails loudly with a clear stack
+// rather than a silent zero-value response.
+type fakeDatastore struct {
+	// Tickets — configurable function fields used by tickets_test.go.
+	listTickets func(*datastores.ListTicketsFilter) ([]*proto.Ticket, string, bool, error)
+	getTicket   func(uint32) (*proto.Ticket, error)
+	getByRef    func(string) (*proto.Ticket, error)
+	firstMsgs   func(uint32, int) ([]*proto.Message, uint32, error)
+	listMsgs    func(uint32, string, uint32, bool) ([]*proto.Message, string, bool, error)
+	listCats    func() ([]*proto.Category, error)
+
+	// Milpacs — configurable function fields. Unset fields nil-deref on call;
+	// same diagnostic level as tickets fields.
+	findProfilesById           func(...uint64) ([]*proto.Profile, error)
+	findProfilesByUsername     func(string) ([]*proto.Profile, error)
+	findProfileByKeycloakID    func(string) (*proto.Profile, error)
+	findProfileByDiscordID     func(string) (*proto.Profile, error)
+	findProfileByGamertag      func(string) (*proto.Profile, error)
+	findRosterByType           func(proto.RosterType) (*proto.Roster, error)
+	findLiteRosterByType       func(proto.RosterType) (*proto.LiteRoster, error)
+	findS1UniformsRosterByType func(proto.RosterType) (*proto.S1UniformsRoster, error)
+}
+
+func (f *fakeDatastore) ListTickets(_ context.Context, _ datastores.TicketReferenceCache, fi *datastores.ListTicketsFilter) ([]*proto.Ticket, string, bool, error) {
+	return f.listTickets(fi)
+}
+func (f *fakeDatastore) GetTicket(_ context.Context, _ datastores.TicketReferenceCache, id uint32, _ string) (*proto.Ticket, error) {
+	return f.getTicket(id)
+}
+func (f *fakeDatastore) GetTicketByRef(_ context.Context, _ datastores.TicketReferenceCache, ref string, _ string) (*proto.Ticket, error) {
+	return f.getByRef(ref)
+}
+func (f *fakeDatastore) GetTicketFirstMessages(_ context.Context, id uint32, n int, _ bool) ([]*proto.Message, uint32, error) {
+	return f.firstMsgs(id, n)
+}
+func (f *fakeDatastore) ListTicketMessages(_ context.Context, id uint32, after string, per uint32, hidden bool) ([]*proto.Message, string, bool, error) {
+	return f.listMsgs(id, after, per, hidden)
+}
+func (f *fakeDatastore) ListCategories(_ context.Context, _ datastores.TicketReferenceCache) ([]*proto.Category, error) {
+	return f.listCats()
+}
+
+func (f *fakeDatastore) FindProfilesById(ids ...uint64) ([]*proto.Profile, error) {
+	return f.findProfilesById(ids...)
+}
+func (f *fakeDatastore) FindProfilesByUsername(u string) ([]*proto.Profile, error) {
+	return f.findProfilesByUsername(u)
+}
+func (f *fakeDatastore) FindRosterByType(t proto.RosterType) (*proto.Roster, error) {
+	return f.findRosterByType(t)
+}
+func (f *fakeDatastore) FindLiteRosterByType(t proto.RosterType) (*proto.LiteRoster, error) {
+	return f.findLiteRosterByType(t)
+}
+func (f *fakeDatastore) FindProfileByKeycloakID(k string) (*proto.Profile, error) {
+	return f.findProfileByKeycloakID(k)
+}
+func (f *fakeDatastore) FindProfileByDiscordID(d string) (*proto.Profile, error) {
+	return f.findProfileByDiscordID(d)
+}
+func (f *fakeDatastore) FindS1UniformsRosterByType(t proto.RosterType) (*proto.S1UniformsRoster, error) {
+	return f.findS1UniformsRosterByType(t)
+}
+func (f *fakeDatastore) FindProfileByGamertag(g string) (*proto.Profile, error) {
+	return f.findProfileByGamertag(g)
+}
+
+// Milpacs methods this PR does not exercise — stay panicking.
+func (f *fakeDatastore) FindProfilesByPosition(string) (*proto.LiteRoster, error)         { panic("unused") }
+func (f *fakeDatastore) FindAllRanks() ([]*proto.RankExpanded, error)                     { panic("unused") }
+func (f *fakeDatastore) FindAllPositionGroups() ([]*proto.PositionGroup, error)           { panic("unused") }
+func (f *fakeDatastore) FindAwol() ([]*proto.Awol, error)                                 { panic("unused") }
+func (f *fakeDatastore) GetTableUpdates() ([]xenforo.TableInfo, error)                    { panic("unused") }
+func (f *fakeDatastore) ValidateApiKey(string) (*datastores.ApiKeyResult, error)          { panic("unused") }
+
+// makeKeyCtx builds a context carrying an ApiKeyResult with the given scope
+// set. The withTicketsKey / withMilpacsKey wrappers stay as named entry
+// points so a test reader sees which handler family the scope applies to.
+func makeKeyCtx(scopes ...string) context.Context {
+	m := map[string]struct{}{}
+	for _, s := range scopes {
+		m[s] = struct{}{}
+	}
+	return ContextWithKey(context.Background(), &datastores.ApiKeyResult{Scopes: m})
+}
+
+// withTicketsKey builds a context carrying an ApiKeyResult with the given
+// scope set. Mirrors the auth middleware's behavior in production.
+func withTicketsKey(scopes ...string) context.Context { return makeKeyCtx(scopes...) }
+
+// withMilpacsKey is the milpacs-handler equivalent of withTicketsKey.
+// Pass "read" to satisfy RequireScope; pass nothing for permission-denied paths.
+func withMilpacsKey(scopes ...string) context.Context { return makeKeyCtx(scopes...) }

servers/grpc/grpc.go

@@ -21,13 +21,15 @@ package grpc
 import (
 	"context"
 	"errors"
+	"log"
+	"os"
+
 	"github.com/7cav/api/datastores"
 	"github.com/7cav/api/proto"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/emptypb"
-	"log"
-	"os"
+	"gorm.io/gorm"
 )
 
 type MilpacsService struct {
@@ -52,16 +54,22 @@ func (server *MilpacsService) GetProfile(ctx context.Context, request *proto.Pro
 		Info.Println("GetProfile, Requested via username")
 		profiles, err = server.Datastore.FindProfilesByUsername(request.Username)
 		if err != nil {
-			return &proto.Profile{}, status.Errorf(codes.NotFound, "no profile found for username: %s", request.Username)
+			if errors.Is(err, gorm.ErrRecordNotFound) {
+				return nil, status.Errorf(codes.NotFound, "no profile found for username: %s", request.Username)
+			}
+			return nil, status.Errorf(codes.Internal, "fetch profile by username: %v", err)
 		}
 	} else if request.UserId != 0 {
 		Info.Println("GetProfile, requested via userid")
 		profiles, err = server.Datastore.FindProfilesById(request.UserId)
 		if err != nil {
-			return &proto.Profile{}, status.Errorf(codes.NotFound, "no profile found for user ID: %d", request.UserId)
+			if errors.Is(err, gorm.ErrRecordNotFound) {
+				return nil, status.Errorf(codes.NotFound, "no profile found for user ID: %d", request.UserId)
+			}
+			return nil, status.Errorf(codes.Internal, "fetch profile by user id: %v", err)
 		}
 	} else {
-		return &proto.Profile{}, status.Errorf(codes.InvalidArgument, "no username or user ID provided")
+		return nil, status.Errorf(codes.InvalidArgument, "no username or user ID provided")
 	}
 
 	return profiles[0], nil
@@ -78,7 +86,7 @@ func (server *MilpacsService) GetRoster(ctx context.Context, request *proto.Rost
 	roster, err := server.Datastore.FindRosterByType(request.Roster)
 
 	if err != nil {
-		return &proto.Roster{}, status.Errorf(codes.NotFound, "no roster found for %s", request.Roster)
+		return nil, status.Errorf(codes.Internal, "fetch roster %s: %v", request.Roster, err)
 	}
 
 	return roster, nil
@@ -96,7 +104,10 @@ func (server *MilpacsService) GetUserViaKeycloakId(ctx context.Context, request
 	profile, err := server.Datastore.FindProfileByKeycloakID(request.GetKeycloakId())
 
 	if err != nil {
-		return &proto.Profile{}, status.Errorf(codes.NotFound, "no user found for keycloakid: %s", request.GetKeycloakId())
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			return nil, status.Errorf(codes.NotFound, "no user found for keycloakid: %s", request.GetKeycloakId())
+		}
+		return nil, status.Errorf(codes.Internal, "fetch profile by keycloak id: %v", err)
 	}
 
 	return profile, nil
@@ -114,7 +125,10 @@ func (server *MilpacsService) GetUserViaDiscordId(ctx context.Context, request *
 	profile, err := server.Datastore.FindProfileByDiscordID(request.GetDiscordId())
 
 	if err != nil {
-		return &proto.Profile{}, status.Errorf(codes.NotFound, "no user found for discordid: %s", request.GetDiscordId())
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			return nil, status.Errorf(codes.NotFound, "no user found for discordid: %s", request.GetDiscordId())
+		}
+		return nil, status.Errorf(codes.Internal, "fetch profile by discord id: %v", err)
 	}
 
 	return profile, nil
@@ -130,7 +144,7 @@ func (server *MilpacsService) GetLiteRoster(ctx context.Context, request *proto.
 	roster, err := server.Datastore.FindLiteRosterByType(request.Roster)
 
 	if err != nil {
-		return &proto.LiteRoster{}, status.Errorf(codes.NotFound, "no roster found for %s", request.Roster)
+		return nil, status.Errorf(codes.Internal, "fetch lite roster %s: %v", request.Roster, err)
 	}
 
 	return roster, nil
@@ -162,7 +176,7 @@ func (server *MilpacsService) GetS1UniformsRoster(ctx context.Context, request *
 	roster, err := server.Datastore.FindS1UniformsRosterByType(request.Roster)
 
 	if err != nil {
-		return &proto.S1UniformsRoster{}, status.Errorf(codes.NotFound, "no roster found for %s", request.Roster)
+		return nil, status.Errorf(codes.Internal, "fetch s1 uniforms roster %s: %v", request.Roster, err)
 	}
 	return roster, nil
 }
@@ -215,13 +229,16 @@ func (server *MilpacsService) GetGamertagProfile(ctx context.Context, request *p
 	}
 	if request.GetGamertag() == "" {
 		Warn.Println("Empty Gamertag provided, cannot return profile")
-		return &proto.Profile{}, status.Errorf(codes.InvalidArgument, "gamertag cannot be empty")
+		return nil, status.Errorf(codes.InvalidArgument, "gamertag cannot be empty")
 	}
 
 	profile, err := server.Datastore.FindProfileByGamertag(request.GetGamertag())
 
 	if err != nil {
-		return &proto.Profile{}, status.Errorf(codes.NotFound, "no user found for gamertag: %v", request.GetGamertag())
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			return nil, status.Errorf(codes.NotFound, "no user found for gamertag: %v", request.GetGamertag())
+		}
+		return nil, status.Errorf(codes.Internal, "fetch profile by gamertag: %v", err)
 	}
 	return profile, nil
 }