WEB-860: Present the GlobalisationCode and translated it to the local languages as part of the message instead of the fixed value for errors#3396
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Error handling service src/app/core/error-handler/error-handler.service.ts |
Injects TranslateService; adds exported interfaces FineractErrorDetail and FineractErrorResponse; adds translateFineractError(errorResponse) to build translated, deduplicated messages and private getMessageForCode(code, defaultMessage) for translation fallback. |
HTTP interceptor src/app/core/http/error-handler.interceptor.ts |
Injects ErrorHandlerService; computes translatedErrorMessage via translateFineractError(...); updates status-to-alert mapping to use translated messages, preserves developerMessage logging in non-production, and silences certain client image 404s. |
Translation files src/assets/translations/{cs-CS,de-DE,en-US,es-CL,es-MX,fr-FR,it-IT,ko-KO,lt-LT,lv-LV,ne-NE,pt-PT,sw-SW}.json |
Adds two new keys in each locale: error.msg.platform.service.unavailable and error.msg.database.type.not.supported with localized messages. |
Sequence Diagram
sequenceDiagram
participant Client as Client/HTTP Request
participant Interceptor as Error Interceptor
participant ErrorService as ErrorHandlerService
participant Translate as TranslateService
participant Alert as AlertService
Client->>Interceptor: receives HTTP Error Response
Interceptor->>ErrorService: translateFineractError(errorBody)
ErrorService->>ErrorService: parse top-level and nested errors, dedupe messages
ErrorService->>Translate: instant(translationKey) or use defaultMessage
Translate-->>ErrorService: translated strings
ErrorService-->>Interceptor: translatedErrorMessage
Interceptor->>Alert: show translatedErrorMessage (alert type based on status)
Alert-->>Client: display error to user
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- Web 429 implement centralized error handler service for user friendly api error messaging #2810: Modifies the same ErrorHandlerService and interceptor usage related to TranslateService integration and message composition.
Suggested reviewers
- IOhacker
- alberto-art3ch
- gkbishnoi07
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Title check | ✅ Passed | The title clearly describes the main change: extracting and presenting GlobalisationCode errors translated to local languages instead of generic fixed messages. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
📝 Coding Plan
- Generate coding plan for human review comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
Tip
You can disable sequence diagrams in the walkthrough.
Disable the reviews.sequence_diagrams setting to disable sequence diagrams in the walkthrough.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/app/core/error-handler/error-handler.service.ts (1)
200-235: Replaceanywith a typed Fineract error contract.The new translator path sits on core error handling; using
anyhere reduces safety and propagates weak typing downstream.♻️ Suggested typed refactor
+interface FineractErrorItem { + userMessageGlobalisationCode?: string; + defaultUserMessage?: string; +} + +interface FineractErrorResponse extends FineractErrorItem { + errors?: FineractErrorItem[]; +} + - translateFineractError(errorResponse: any): string { + translateFineractError(errorResponse: FineractErrorResponse | null | undefined): string { if (!errorResponse || typeof errorResponse !== 'object') { return ''; } @@ - if (Array.isArray(errorResponse.errors)) { - errorResponse.errors.forEach((error: any) => { + if (Array.isArray(errorResponse.errors)) { + errorResponse.errors.forEach((error: FineractErrorItem) => { if (!error || typeof error !== 'object') { return; }Based on learnings: “In TypeScript files … introduce specific interfaces/types for the response shapes and use proper typing instead of any.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/error-handler/error-handler.service.ts` around lines 200 - 235, translateFineractError currently accepts untyped errorResponse and uses any for nested errors; define a FineractErrorResponse interface (fields: userMessageGlobalisationCode?: string, defaultUserMessage?: string, errors?: FineractErrorDetail[]) and a FineractErrorDetail interface (same optional fields) in this file or a shared types file, then change the function signature to translateFineractError(errorResponse: FineractErrorResponse | null | undefined) and update the forEach callback to use (error: FineractErrorDetail) so all property accesses are type-checked while keeping the existing logic and still calling getMessageForCode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 50-55: The code currently includes errorBody.developerMessage in
the UI errorMessage; remove developerMessage from the user-facing message
assembly in error-handler.interceptor.ts and only surface translatedErrorMessage
or errorBody.defaultUserMessage (or response.message as final fallback) when
constructing errorMessage for UI. If developerMessage exists, ensure it's
recorded to logs only (e.g., send errorBody.developerMessage to the existing
logging path or errorHandlerService) rather than concatenating it into
errorMessage; keep translateFineractError(errorBody) and
errorBody.defaultUserMessage as the primary user-facing sources and log
developerMessage separately.
- Around line 90-93: Replace hardcoded English strings in the
ErrorHandlerInterceptor's alertService.alert calls with i18n lookups using
this.translate.instant; specifically change the alert type and message for
Internal Server Error to
this.translate.instant('error.resource.internalServerError.type') and
this.translate.instant('error.resource.internalServerError.message') (and apply
the same pattern to other status branches such as 401, 403, 400 and fallback 404
messages that currently use literal strings), updating the alertService.alert
invocations in the error-handler.interceptor (e.g., inside the handleError /
intercept branches) to use translate.instant keys, then run npm run
translations:extract to add the new keys to translation files.
In `@src/assets/translations/es-CL.json`:
- Line 6: Update the translation value for the key
"error.msg.database.type.not.supported" to use singular phrasing and include
terminal punctuation; locate that key in src/assets/translations/es-CL.json and
change the string from "El tipo de datos no es compatible" to a natural singular
form with a period at the end (e.g., "El tipo de dato no es compatible.").
In `@src/assets/translations/ne-NE.json`:
- Around line 5-6: The Nepali locale entries are written with Hindi-leaning
phrasing; update the two keys "error.msg.platform.service.unavailable" and
"error.msg.database.type.not.supported" to proper Nepali wording (e.g., use
phrases like "अनुरोध प्रस्तुत गर्न सक्षम छैन" / "कृपया केही समयपछि पुन: प्रयास
गर्नुहोस्" for the platform outage message and "डेटाबेस प्रकार समर्थन हुँदैन" or
"डेटाबेस प्रकार समर्थन गरिंदैन" for the unsupported DB message) so the ne-NE
file uses consistent Nepali phrasing instead of Hindi terms.
---
Nitpick comments:
In `@src/app/core/error-handler/error-handler.service.ts`:
- Around line 200-235: translateFineractError currently accepts untyped
errorResponse and uses any for nested errors; define a FineractErrorResponse
interface (fields: userMessageGlobalisationCode?: string, defaultUserMessage?:
string, errors?: FineractErrorDetail[]) and a FineractErrorDetail interface
(same optional fields) in this file or a shared types file, then change the
function signature to translateFineractError(errorResponse:
FineractErrorResponse | null | undefined) and update the forEach callback to use
(error: FineractErrorDetail) so all property accesses are type-checked while
keeping the existing logic and still calling getMessageForCode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5aa5672b-efe1-48cc-a2e5-2e9a04cb022d
📒 Files selected for processing (15)
src/app/core/error-handler/error-handler.service.tssrc/app/core/http/error-handler.interceptor.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
ece690d to
908d688
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 69-73: The branch that detects an invalid one-time token should
not rely on the localized `errorMessage` string; update the conditional in
error-handler.interceptor (the block using `status === 403` and `errorMessage
=== 'The provided one time token is invalid'`) to instead check for a stable
backend identifier such as `body.userMessageGlobalisationCode` (or
`body.defaultUserMessage` as a fallback) before calling
`this.alertService.alert(...)`; ensure you still handle the case where those
fields are absent by falling back to the existing localized `errorMessage` check
only as last resort so non-English sessions route correctly.
- Around line 97-105: The current 500/501 branches call alertService.alert with
only the generic translate.instant messages, dropping the already-computed
translated backend detail (errorMessage) created earlier (see the errorMessage
computation). Update the 500 and 501 branches in error-handler.interceptor.ts so
that alertService.alert still uses the translated title via
translate.instant(...) for type but uses the computed errorMessage (or falls
back to the generic translate.instant message) for message; reference the
alertService.alert calls, the translate.instant invocations, and the
errorMessage variable to locate and adjust the logic accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a2abf84-9834-411d-9a2a-51292045d180
📒 Files selected for processing (15)
src/app/core/error-handler/error-handler.service.tssrc/app/core/http/error-handler.interceptor.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (9)
- src/assets/translations/es-CL.json
- src/assets/translations/es-MX.json
- src/assets/translations/lv-LV.json
- src/assets/translations/it-IT.json
- src/assets/translations/pt-PT.json
- src/assets/translations/de-DE.json
- src/assets/translations/lt-LT.json
- src/assets/translations/ne-NE.json
- src/app/core/error-handler/error-handler.service.ts
|
Which is the ticket? |
https://mifosforge.jira.com/issues?filter=-1&selectedIssue=WEB-860 |
IOhacker
left a comment
There was a problem hiding this comment.
@EDSONZ-WASSWA please change the title of the PR as "WEB-860: Present the GlobalisationCode and translated it to the local languages as part of the message instead of the fixed value for errors"
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/assets/translations/ko-KO.json`:
- Around line 5-6: This file fails the project's Prettier formatting check; run
the formatter (e.g., npx prettier --write .) to normalize spacing, trailing
commas/newline and overall JSON style so the translations remain valid for
ErrorHandlerService.getMessageForCode() lookups; after formatting, verify the
JSON still contains the keys "error.msg.platform.service.unavailable" and
"error.msg.database.type.not.supported" at the root and commit the formatted
file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa323809-3c28-48d2-9f3d-2ba8827640db
📒 Files selected for processing (13)
src/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (11)
- src/assets/translations/lt-LT.json
- src/assets/translations/lv-LV.json
- src/assets/translations/cs-CS.json
- src/assets/translations/es-MX.json
- src/assets/translations/en-US.json
- src/assets/translations/fr-FR.json
- src/assets/translations/sw-SW.json
- src/assets/translations/ne-NE.json
- src/assets/translations/de-DE.json
- src/assets/translations/pt-PT.json
- src/assets/translations/es-CL.json
…uages as part of the message modified: src/app/core/error-handler/error-handler.service.ts modified: src/app/core/http/error-handler.interceptor.ts
8071ddd to
a8fe562
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/app/core/http/error-handler.interceptor.ts (1)
105-114:⚠️ Potential issue | 🟠 Major500/501 branches still drop translated backend detail.
These branches always show generic text and ignore the computed
errorMessage, which can regress the detailed translated-error behavior introduced by this PR.💡 Proposed fix
} else if (status === 500) { this.alertService.alert({ type: this.translate.instant('error.resource.internalServerError.type'), - message: this.translate.instant('error.resource.internalServerError.message') + message: errorMessage || this.translate.instant('error.resource.internalServerError.message') }); } else if (status === 501) { this.alertService.alert({ type: this.translate.instant('error.resource.notImplemented.type'), - message: this.translate.instant('error.resource.notImplemented.message') + message: errorMessage || this.translate.instant('error.resource.notImplemented.message') });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` around lines 105 - 114, The 500 and 501 branches in the error handler (the status === 500 and status === 501 blocks) always set the alert message to a generic translate.instant string and ignore the previously computed errorMessage; update those branches in handleError/error-handler.interceptor to use the computed errorMessage (or combine the translated fallback with errorMessage) when calling alertService.alert so translated backend details are preserved (keep using translate.instant for the type field).
🧹 Nitpick comments (1)
src/app/core/http/error-handler.interceptor.ts (1)
50-53: AdddeveloperMessageto theFineractErrorResponseinterface and remove theanytype annotation.The
errorBodyvariable usesanytype, which bypasses strict typing. The code accessesdeveloperMessage(line 52) anddefaultUserMessage(line 53), but theFineractErrorResponseinterface only includesdefaultUserMessage, notdeveloperMessage. This gap should be fixed at the interface definition level rather than with a workaround.Update
FineractErrorResponseinsrc/app/core/error-handler/error-handler.service.tsto include the missing property:export interface FineractErrorResponse { userMessageGlobalisationCode?: string; defaultUserMessage?: string; developerMessage?: string; errors?: FineractErrorDetail[]; }Then change line 50 from
const errorBody: any = response.error;toconst errorBody: FineractErrorResponse | null = response.error || null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` around lines 50 - 53, Update the FineractErrorResponse type and stop using any for errorBody: add developerMessage?: string to the FineractErrorResponse interface in error-handler.service.ts (keeping existing userMessageGlobalisationCode?, defaultUserMessage?, errors?: FineractErrorDetail[]), then in error-handler.interceptor.ts change the errorBody declaration to be typed as FineractErrorResponse | null (e.g., const errorBody: FineractErrorResponse | null = response.error || null) so developerMessage and defaultUserMessage are strongly typed when accessed by translateFineractError and the subsequent developerMessage/errorMessage logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 105-114: The 500 and 501 branches in the error handler (the status
=== 500 and status === 501 blocks) always set the alert message to a generic
translate.instant string and ignore the previously computed errorMessage; update
those branches in handleError/error-handler.interceptor to use the computed
errorMessage (or combine the translated fallback with errorMessage) when calling
alertService.alert so translated backend details are preserved (keep using
translate.instant for the type field).
---
Nitpick comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 50-53: Update the FineractErrorResponse type and stop using any
for errorBody: add developerMessage?: string to the FineractErrorResponse
interface in error-handler.service.ts (keeping existing
userMessageGlobalisationCode?, defaultUserMessage?, errors?:
FineractErrorDetail[]), then in error-handler.interceptor.ts change the
errorBody declaration to be typed as FineractErrorResponse | null (e.g., const
errorBody: FineractErrorResponse | null = response.error || null) so
developerMessage and defaultUserMessage are strongly typed when accessed by
translateFineractError and the subsequent developerMessage/errorMessage logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66c2f2a6-f8c0-4238-ad83-2e054d1c2771
📒 Files selected for processing (15)
src/app/core/error-handler/error-handler.service.tssrc/app/core/http/error-handler.interceptor.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (9)
- src/assets/translations/it-IT.json
- src/assets/translations/ko-KO.json
- src/assets/translations/cs-CS.json
- src/assets/translations/lv-LV.json
- src/assets/translations/fr-FR.json
- src/assets/translations/pt-PT.json
- src/assets/translations/sw-SW.json
- src/assets/translations/de-DE.json
- src/assets/translations/ne-NE.json
…uages as part of the message
Description
Before, the Mifos X Web App showed a generic message like “Unknown Error. Please try again later” whenever an API call failed. Even though Apache Fineract returned detailed error information, the UI ignored it, making it hard for users to understand what went wrong.
Now, the app extracts those backend error codes, translates them, and displays a clear, meaningful message. This makes errors easier to understand and much simpler to debug.
Related issues and discussion
#{Issue Number}
https://mifosforge.jira.com/issues?filter=-1&selectedIssue=WEB-860
Screenshots, if any
before



After
please @IOhacker please have a look at this
Summary by CodeRabbit
Improvements
New Translation Strings