feat(i18n): localised error messages (en, id) via Accept-Language (Tier-B #6)#14
Merged
feat(i18n): localised error messages (en, id) via Accept-Language (Tier-B #6)#14
Conversation
Adds pkg/i18n with: - Bundle (immutable code→locale→message catalogue) - Locale (BCP-47 primary subtag normaliser) - DefaultBundle() pre-loaded with EN + ID translations for every error code emitted by the framework's built-in modules (auth, validator, RBAC, menu, rate-limit, route resolution) - Fiber middleware that parses Accept-Language and stores the chosen locale on c.UserContext() - Process-wide SetGlobal/Global so httpx.RespondError can translate without taking a Bundle parameter httpx.RespondError now calls i18n.T(ctx, code, payload.Message) so error messages are translated by code with the original message as fallback. Apps that don't wire i18n keep their existing strings. Wired into internal/app/app.go (SetGlobal at boot) and internal/infrastructure/server/server.go (middleware before CORS so rate-limit and timeout errors are translated too). go test -race ./pkg/i18n/... clean; golangci-lint clean. Co-Authored-By: dede febriansyah <febriansyahd65@gmail.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin Review flagged that the original design introduced new package-
level mutable state (globalBundle / SetGlobal / Global / T) which
violates AGENTS.md rule 2 ('No new global state. Pass services as
constructor arguments') and CONTRIBUTING.md rule 3.
Refactor:
- i18n.Middleware now accepts a *Bundle and stores both the
bundle and the resolved locale on c.UserContext().
- i18n.T(ctx, code, fallback) reads the bundle and locale from
ctx; with no bundle attached, it returns the fallback unchanged
so apps that don't wire i18n keep their existing English
messages (the same nil-safe behaviour as before, now without a
package singleton).
- server.New(cfg, log, bundle) takes the bundle explicitly. The
composition root in internal/app/app.go constructs it once
(i18nBundle := i18n.DefaultBundle()) and threads it down.
- SetGlobal/Global/the implicit-global T are gone.
Tests updated; all six i18n tests pass without touching any global.
go test -race ./... and golangci-lint run ./... clean.
Co-Authored-By: dede febriansyah <febriansyahd65@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…verage test
Devin Review correctly flagged that DefaultBundle registered
translations under simplified placeholder codes (permission.not_found,
role.taken, ...) that don't match the actual codes the domain layer
emits via errs.NotFound/Conflict/Forbidden (rbac.permission_not_found,
rbac.role_code_taken, rbac.role_is_system, ...). The Indonesian
translations were therefore dead code: i18n.T would miss the bundle
lookup and silently fall back to the English message stored on the
errs.Error itself.
Fix:
- Rename every miscoded entry in DefaultBundle to match the domain.
- Backfill the codes I had missed entirely (auth.invalid_subject,
auth.wrong_token_kind, auth.token_reused, jwt.invalid, user.not_found,
tenant.missing/invalid, idempotency.key_too_long/key_reused,
every rbac.* code emitted by internal/usecase/rbac.go, every
menu.* code emitted by internal/usecase/menu.go).
- Add a regression test (pkg/i18n/bundle_default_test.go) that
walks internal/ and pkg/ at test-time, extracts every literal
code passed to a client-facing errs constructor, and fails if
any of them are missing from DefaultBundle. errs.Wrap is
intentionally excluded because those codes never reach the
client - they're masked to the generic 'internal' payload by
httpx.RespondError. With this guard, future code that adds new
user-facing error codes without registering translations will
fail the build, not silently fall back to English.
- Update the existing TestDefaultBundle_HasShippedCodes smoke
list to use the real codes (rbac.permission_code_taken,
rbac.role_is_system) instead of the placeholders.
go test -race ./... clean.
Co-Authored-By: dede febriansyah <febriansyahd65@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Three findings, all real, all fixed:
1) [CRITICAL] auth.wrong_token_kind reused for opposite scenarios
The same code was emitted from two opposite places:
- middleware/auth.go:30 (refresh sent to a protected endpoint)
message: "access token required"
- usecase/auth.go:170 (access sent to /refresh)
message: "refresh token required"
The i18n bundle maps one code to one message, so once the bundle
replaced the dynamic message both scenarios collapsed onto the
same translation - actively misleading the API consumer (telling
them "access token required" when they should send a refresh,
or vice versa).
Fix: split into two distinct codes.
- auth.wrong_token_kind (kept; access token required)
- auth.refresh_token_required (new; refresh token required)
Bundle gets entries for both in EN+ID. Documented inline so future
contributors do not collapse them again.
2) [Medium] Dynamic permission code lost after i18n translation
permission.go used to build the message
"missing required permission: " + code
so the API consumer could see *which* permission was missing.
After i18n the message is replaced by the static bundle entry and
the dynamic part disappears, regressing debuggability.
Fix: stash the missing permission code in errs.Meta instead of
concatenating it into the human-readable message. The translated
message stays a clean human sentence; the consumer reads
meta.permission (single-perm gate) or meta.permissions_any_of
(any-of gate) for the structured detail.
3) [Medium] pickLocale dropped Accept-Language values with whitespace
RFC 9110 allows optional whitespace before ";", so a perfectly
valid header "id ;q=0.8" used to fall through silently because
item[:i] preserved the trailing space, and Locale("id ") never
matched the supported set. Tags that *happened* to contain a "-"
("en-US ;q=0.9") worked by accident because Normalise() strips
at the dash.
Fix: TrimSpace the candidate after stripping the q-suffix.
Regression test middleware_test.go covers bare + region + multi-
value + no-match cases.
go test -race ./... clean, golangci-lint clean.
Co-Authored-By: dede febriansyah <febriansyahd65@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin Review on the previous fix correctly noted that the bundle
coverage scanner would silently miss any constructor formatted
across lines. After the rbac.permission_required reflow:
return httpx.RespondError(c, errs.
Forbidden("rbac.permission_required", "missing required permission").
With("permission", code))
the original regex (which assumed errs.Foo on a single line) fell
through to a no-op and would not have caught a future regression
where somebody removed the bundle entry.
Two changes to the regex:
1. (?s) flag so '.' matches newlines.
2. \s* between 'errs.' and the constructor name covers the
opposite reformatting (the dot kept on the previous line).
3. [^"]*? for the gap between '(' and the first string literal,
so any future formatting (extra args, line breaks, comments)
stays matched without churning the test on every reflow.
Verified by a one-shot scan: 46 codes discovered, including
rbac.permission_required which was missed before this fix.
go test -race ./... clean.
Co-Authored-By: dede febriansyah <febriansyahd65@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin Review on PR #14 caught a real semantic mismatch. The only emitter of auth.invalid is internal/usecase/auth.go:184 inside the ErrUnknownToken branch, which fires when the refresh token is unknown to the rotation store - i.e. revoked or already consumed. Expired tokens are caught earlier by uc.tokens.Parse and surface as jwt.invalid. Old translations said 'expired'/'kedaluwarsa', misleading any client whose retry logic distinguishes 'try again with a fresh login' (revoked) from 'try with the existing access token' (expired). Applied the suggested copy verbatim and added an inline comment on the bundle entry so future edits do not reintroduce the mismatch. go test -race ./pkg/i18n/... clean. Co-Authored-By: dede febriansyah <febriansyahd65@gmail.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.
Summary
Adds
pkg/i18n— a small, dependency-free message bundle with EN + ID translations for every error code the framework emits. Error responses are translated automatically based on the request'sAccept-Language. Apps that don't wire i18n keep their existing English messages — the integration is opt-out by default and zero-cost on the hot path when no global bundle is set.Why
Tier-B #6 from the framework-improvement plan. You're an Indonesian-language user; every project built on goforge will eventually need to render errors and validation messages in
id. Solving this once at the framework level means downstream projects don't reinvent locale detection, fallback chains, or message catalogues.How
pkg/i18n/i18n.go—Locale,Bundle(RWMutex-protected map),WithLocale/FromContextfor ctx routing, process-wideSetGlobal/Globalso packages without explicit DI (httpx) can still translate.T(ctx, code, fallback)is the only call site needed by callers.pkg/i18n/bundle_default.go—DefaultBundle()pre-loaded with translations forinternal,validation,request.*,auth.*,user.*,rate_limited,route.*,permission.*,role.*,menu.*. Easy to extend viaAdd/AddMany.pkg/i18n/middleware.go— Fiber middleware that parsesAccept-Language, picks the first supported locale (primary subtag only —en-USreduces toen), and stores it onc.UserContext().pkg/httpx/response.go—RespondErrornow callsi18n.T(ctx, code, payload.Message). The original message is the fallback, so apps that don't wire i18n see no behavioural change.internal/app/app.go— pinsDefaultBundle()viai18n.SetGlobalat boot (before any handler runs).internal/infrastructure/server/server.go— registers the middleware right afterRequestIDand before CORS / rate-limit, so even rate-limit errors are localised.Test plan
go test -race ./pkg/i18n/...— happy path, fallback, normalisation, Accept-Language picker (incl.*, q-values, unsupported), default-bundle code coveragego build ./...cleangolangci-lint run ./...cleancurl -H 'Accept-Language: id' -X POST /api/v1/auth/login -d '{}'returns Indonesian validation message; same call without the header returns EnglishRisk
Accept-Language: idwill now receive Indonesian messages. Frontends that switched onerror.messagestrings to display localised UI text should switch toerror.codeinstead — that's the stable contract. The README already documents this.i18n.SetGlobal(custom)and restore viadefer i18n.SetGlobal(nil).Checklist
fmt.Printlnleft behindLink to Devin session: https://app.devin.ai/sessions/8fdfc20358514c97a766adca630a2527
Requested by: @dedeez14