fix(grpc): MilpacsService error-handling cleanups (#98)#107
Merged
Conversation
GetRoster / GetLiteRoster / GetS1UniformsRoster rejected ROSTER_TYPE_UNSPECIFIED with a bare errors.New, which grpc-gateway maps to codes.Unknown -> HTTP 500. A bad enum from the consumer is the canonical 400 case; swap to status.Errorf(codes.InvalidArgument, ...). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetUserViaKeycloakId and GetUserViaDiscordId logged a warning then fell through to the datastore lookup with an empty string, yielding a 404 or 500. Mirror GetGamertagProfile: early-return status.Errorf(codes.InvalidArgument, ...) so the consumer gets a 400 and no datastore call is made. KeycloakId path is soft-deprecated (see CLAUDE.md); cleaned in place, no new dependency added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FindProfileByKeycloakID, FindProfileByDiscordID, and the S1-uniforms
inline loop in FindS1UniformsRosterByType returned
fmt.Errorf("error generating profile") with no %w, unlike the other
finders. Wrap the underlying err so errors.Is works across the board.
No observable consumer change today (handler maps all to codes.Internal).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #98
Three adjacent error-handling cleanups in
MilpacsService, one commit each:fix(grpc)null roster type —GetRoster/GetLiteRoster/GetS1UniformsRosternow returnstatus.Errorf(codes.InvalidArgument, ...)instead of bareerrors.New, so grpc-gateway maps to HTTP 400 (not 500) onROSTER_TYPE_UNSPECIFIED.fix(grpc)empty keycloak/discord id —GetUserViaKeycloakId/GetUserViaDiscordIdearly-returncodes.InvalidArgumentinstead of warn-and-fall-through with an empty string, mirroringGetGamertagProfile.chore(datastores)%w wrap —FindProfileByKeycloakID,FindProfileByDiscordID, and theFindS1UniformsRosterByTypeinline loop now wrap witherror generating profile: %w, matching the other finders. Enables futureerrors.Is.Notes / deviations
GetGamertagProfilealready had the early-return pattern (triage comment was stale) — left unchanged, added a regression-guard test.codes.Internal); verified via grep (zero bare-string variants remain) + stdlib%wguarantee rather than a fragile sqlmock test.GetUserViaKeycloakIdis soft-deprecated — cleanup applied in place, no new dependencies added.Tests
servers/grpc/milpacs_test.go: +6 behavioral tests (3 null-roster-type, 3 empty-id →InvalidArgument).Gate
build ✅ ·
go test ./...✅ ·buf lint+buf breaking --against develop✅🤖 Generated with Claude Code