fix: WEB-862 Localise the error as per the selected language#3407
fix: WEB-862 Localise the error as per the selected language#3407saksham869 wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Error Handler src/app/core/http/error-handler.interceptor.ts |
Rewrote error parsing to be null-safe, extract globalisationCode from nested/top-level response.error, use TranslateService.instant() (apply translation only if resolved), replace EMPTY image handling with an Observable emitting null, add explicit 'error.token.invalid' 403 branch, and update alert branches for 400/401/403/500/501/default to prefer translated type/message. |
i18n — English src/assets/translations/en-US.json |
Added 14 new top-level error.*.{type,message} translation keys (authentication, token invalid, bad request, unauthorized, resource not found, internal server, unknown). |
i18n — Other locales src/assets/translations/{cs-CS,de-DE,es-CL,es-MX,fr-FR,it-IT,ko-KO,lt-LT,lv-LV,ne-NE,pt-PT,sw-SW}.json |
Added the same set of 14 localized error.*.{type,message} keys across many locales; some files also removed older keys like linkedSavingsAccountOwnership or error.resource.notImplemented.* where noted. |
Environment src/environments/.env.ts |
Bumped exported mifos_x.version from '260108' → '260318' and mifos_x.hash from 'g5af11fe75' → 'gd4fa50454'. |
Sequence Diagram(s)
mermaid
sequenceDiagram
participant Client
participant HTTP as HTTP Request
participant Interceptor as ErrorHandlerInterceptor
participant Translate as TranslateService
participant Alert as AlertService
participant App as Application
Client->>HTTP: send request
HTTP->>Client: response (error)
HTTP->>Interceptor: intercept error response
Interceptor->>Interceptor: parse response.error (safe access)
alt has globalisationCode
Interceptor->>Translate: instant(globalisationCode)
Translate-->>Interceptor: translation (or key)
alt translation differs from key
Interceptor->>Alert: show translated type/message
else
Interceptor->>Alert: show fallback errorMessage
end
else no globalisationCode
Interceptor->>Alert: show fallback errorMessage
end
Interceptor->>App: propagate (null or rethrow)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- WEB-456: Add exception message for HTTP 501 #2840: Modifies the same
error-handler.interceptor.tsto add 501 handling and TranslateService-driven messages — directly related. - WEB-521 fix(i18n): add missing translations and apply translate pipes across … #2934: Adds overlapping i18n error keys and TranslateService usage patterns relevant to these translation additions.
Suggested reviewers
- IOhacker
- alberto-art3ch
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The PR title clearly summarizes the main change: implementing localization of error messages according to the selected language, which aligns with the core objective of the changeset. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ 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.
saksham869
force-pushed
the
fix/WEB-862-final
branch
from
0624a89 to
f656f64
Compare
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
@saksham869 please review that only one commit is expected and make sure that the title is following the syntax required
saksham869
changed the title
saksham869
force-pushed
the
fix/WEB-862-final
branch
2 times, most recently
from
1d90eb5 to
2c1f4a9
Compare
saksham869
force-pushed
the
fix/WEB-862-final
branch
from
2c1f4a9 to
196f3e0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/assets/translations/pt-PT.json (1)
11-24: Consider normalizing the 501 key naming style for consistency.New keys use dotted segments (e.g.,
error.resource.not.found.*), while the nearby 501 key useserror.resource.notImplemented.*. Keeping both styles aligned reduces future lookup mismatches.♻️ Optional consistency patch
+ "error.resource.not.implemented.type": "Erro não implementado", + "error.resource.not.implemented.message": "Funcionalidade não implementada!",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/pt-PT.json` around lines 11 - 24, The 501-related keys use a different naming style (e.g., error.resource.notImplemented.type/message) which conflicts with the dotted-segment convention used elsewhere (e.g., error.resource.not.found.type/message); rename the 501 keys to follow the dotted convention (for example error.resource.not.implemented.type and error.resource.not.implemented.message) and update any lookup sites that reference the old keys to use the new keys to avoid runtime lookup mismatches.
🤖 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-64: The interceptor currently lets
response.error.userMessageGlobalisationCode overwrite a more-specific nested
code and may have initialized errorMessage from developerMessage before checking
nested errors; update the logic in error-handler.interceptor.ts so that you
first prefer and assign nested values
(response.error.errors[0].userMessageGlobalisationCode and
.defaultUserMessage/.developerMessage) to globalisationCode and errorMessage,
and only set globalisationCode from response.error.userMessageGlobalisationCode
if globalisationCode is still null/undefined; ensure errorMessage is not
overwritten by a top-level developerMessage once a nested defaultUserMessage was
chosen.
- Around line 133-136: The catch-all else branch currently discards any resolved
backend message and always uses the generic unknown message; update the else
branch in the error-handling flow (the code that calls alertService.alert and
translate.instant) to prefer the resolved backend message (e.g. the
previously-derived defaultUserMessage or the translated
userMessageGlobalisationCode) when present, and only fall back to
translate.instant('error.unknown.message') if no backend message exists; ensure
you use the same translation helper (translate.instant) for the globalization
code path and pass that resolved string into alertService.alert's message field.
- Around line 109-114: The current branch where isClientImage404 returns a
successful Observable with null swallows the original 404; instead rethrow the
original error so downstream image service can handle it. Replace the return new
Observable(...) in the isClientImage404 branch with a rethrow using
throwError(() => err) (or throwError(() => originalError)) so the interceptor
emits the error rather than a null value; keep the isClientImage404 condition
and use the same err reference available in the interceptor's catch block.
- Around line 67-72: In src/app/core/http/error-handler.interceptor.ts the
hardcoded translation keys (e.g., 'error.auth.type', 'error.token.invalid.*',
'error.bad.request.*', etc.) lack the required "errors." prefix and therefore
fail to resolve; update all those hardcoded keys to include the "errors." prefix
(for example 'error.auth.type' -> 'errors.error.auth.type') and mirror the
existing pattern used around globalisationCode: use translate.instant(...) then
verify the returned value !== key before assigning to errorMessage (referencing
globalisationCode, translate.instant and errorMessage in the interceptor) so
only valid translations replace the fallback.
---
Nitpick comments:
In `@src/assets/translations/pt-PT.json`:
- Around line 11-24: The 501-related keys use a different naming style (e.g.,
error.resource.notImplemented.type/message) which conflicts with the
dotted-segment convention used elsewhere (e.g.,
error.resource.not.found.type/message); rename the 501 keys to follow the dotted
convention (for example error.resource.not.implemented.type and
error.resource.not.implemented.message) and update any lookup sites that
reference the old keys to use the new keys to avoid runtime lookup mismatches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5d483fe-52d3-4aaa-8f28-18ad8552bde4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (15)
src/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.jsonsrc/environments/.env.ts
| let errorMessage = response.error?.developerMessage || response.message; | ||
| let globalisationCode: string | null = null; | ||
|
|
||
| // Extract globalisation code and message from Fineract error response | ||
| if (response.error?.errors) { | ||
| if (response.error.errors[0]) { | ||
| globalisationCode = response.error.errors[0].userMessageGlobalisationCode || null; | ||
| errorMessage = response.error.errors[0].defaultUserMessage || response.error.errors[0].developerMessage; | ||
| } | ||
| } | ||
|
|
||
| // Also check top-level userMessageGlobalisationCode | ||
| if (response.error?.userMessageGlobalisationCode) { | ||
| globalisationCode = response.error.userMessageGlobalisationCode; | ||
| } |
There was a problem hiding this comment.
Don't let the top-level error overwrite the nested/user-facing one.
response.error.userMessageGlobalisationCode currently replaces errors[0].userMessageGlobalisationCode, and the initial fallback starts from developerMessage. When both shapes are present, the interceptor can discard the more specific nested translation and fall back to technical English text.
🔧 Suggested fix
- let errorMessage = response.error?.developerMessage || response.message;
+ let errorMessage =
+ response.error?.defaultUserMessage || response.error?.developerMessage || response.message;
let globalisationCode: string | null = null;
@@
- if (response.error?.userMessageGlobalisationCode) {
+ if (!globalisationCode && response.error?.userMessageGlobalisationCode) {
globalisationCode = response.error.userMessageGlobalisationCode;
+ errorMessage = response.error?.defaultUserMessage || errorMessage;
}🤖 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 - 64, The
interceptor currently lets response.error.userMessageGlobalisationCode overwrite
a more-specific nested code and may have initialized errorMessage from
developerMessage before checking nested errors; update the logic in
error-handler.interceptor.ts so that you first prefer and assign nested values
(response.error.errors[0].userMessageGlobalisationCode and
.defaultUserMessage/.developerMessage) to globalisationCode and errorMessage,
and only set globalisationCode from response.error.userMessageGlobalisationCode
if globalisationCode is still null/undefined; ensure errorMessage is not
overwritten by a top-level developerMessage once a nested defaultUserMessage was
chosen.
| if (globalisationCode) { | ||
| const translated = this.translate.instant(globalisationCode, response.error?.errors?.[0] || response.error || {}); | ||
| // Only use translation if the key actually exists (translate returns the key itself if not found) | ||
| if (translated !== globalisationCode) { | ||
| errorMessage = translated; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
# Check whether the app customizes ngx-translate parsing and how existing error keys are referenced.
rg -n -C 3 "TranslateModule\.forRoot|TranslateParser|MissingTranslationHandler|TranslateLoader" src
rg -n -C 1 "errors\.[A-Za-z0-9_.-]+" src/appRepository: openMF/web-app
Length of output: 10304
🏁 Script executed:
cat -n src/app/core/http/error-handler.interceptor.ts | head -140Repository: openMF/web-app
Length of output: 6766
🏁 Script executed:
fd -e json src/assets/translations | head -10Repository: openMF/web-app
Length of output: 455
🏁 Script executed:
head -100 src/assets/translations/lt-LT.jsonRepository: openMF/web-app
Length of output: 11943
🏁 Script executed:
rg -n "instant\('error\." src/app --type ts -A 1 | head -40Repository: openMF/web-app
Length of output: 2854
🏁 Script executed:
rg -n "instant\('errors\." src/app --type ts -A 1 | head -40Repository: openMF/web-app
Length of output: 1099
🏁 Script executed:
grep -r "error\.auth\|errors\.error\.auth" src/assets/translations/Repository: openMF/web-app
Length of output: 2599
🏁 Script executed:
cat src/assets/translations/lt-LT.json | head -20 && cat src/assets/translations/lt-LT.json | tail -5Repository: openMF/web-app
Length of output: 1155
🏁 Script executed:
python3 << 'EOF'
import json
with open('src/assets/translations/lt-LT.json', 'r', encoding='utf-8') as f:
data = json.load(f)
# Check the structure of "errors" top-level keys
errors = data.get('errors', {})
print("Keys under 'errors' object:")
for key in list(errors.keys())[:15]:
print(f" {key}")
if isinstance(errors[key], dict):
for subkey in errors[key].keys():
print(f" {subkey}")
print("\nTotal keys in errors:", len(errors))
EOFRepository: openMF/web-app
Length of output: 556
🏁 Script executed:
cat -n src/app/app.module.ts | sed -n '72,76p'Repository: openMF/web-app
Length of output: 369
🏁 Script executed:
rg -n "getTranslation|onTranslationNotFound" src/app --type ts -B 2 -A 5Repository: openMF/web-app
Length of output: 40
Fix translation key paths in error-handler interceptor.
The hardcoded error keys on lines 84–137 are missing the errors. prefix required to match the nested structure in translation files. For example, 'error.auth.type' should be 'errors.error.auth.type', and similarly for all other hardcoded error keys (error.token.invalid.*, error.bad.request.*, error.unauthorized.*, error.resource.not.found.*, error.server.internal.*, error.resource.notImplemented.*, error.unknown.*). Without this prefix, translate.instant() will fail to resolve the keys and fall back to displaying the key name instead of the translated message. This is inconsistent with other parts of the codebase that correctly use the errors. prefix (e.g., 'errors.validationSaveError', 'errors.accountingRule.duplicateName').
The same approach used on lines 67–72 (verifying that the translation key exists before using it) should also be applied to all hardcoded error keys, or the keys should be prefixed consistently.
🤖 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 67 - 72, In
src/app/core/http/error-handler.interceptor.ts the hardcoded translation keys
(e.g., 'error.auth.type', 'error.token.invalid.*', 'error.bad.request.*', etc.)
lack the required "errors." prefix and therefore fail to resolve; update all
those hardcoded keys to include the "errors." prefix (for example
'error.auth.type' -> 'errors.error.auth.type') and mirror the existing pattern
used around globalisationCode: use translate.instant(...) then verify the
returned value !== key before assigning to errorMessage (referencing
globalisationCode, translate.instant and errorMessage in the interceptor) so
only valid translations replace the fallback.
| if (isClientImage404) { | ||
| // Don't show alerts for missing client images | ||
| // This is an expected condition, not an error | ||
| return EMPTY; | ||
| // Return observable of null for missing client images so imaging service can handle gracefully | ||
| return new Observable((observer) => { | ||
| observer.next(null); | ||
| observer.complete(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Angular HttpInterceptor, if the goal is to let downstream request handlers process a 404, should the interceptor rethrow with throwError(() => response)instead of emittingnullfromintercept()?
💡 Result:
Yes, if the goal is to let downstream request handlers (services, components) process the 404 error via their error callbacks, the interceptor should rethrow using throwError( => response) (or throwError(response) in older RxJS) from within catchError, rather than emitting null (e.g., via of(null)) from intercept. Emitting null or any value completes the observable successfully, so subscribers receive it in the next handler and the error callback is never triggered. Rethrowing propagates the original HttpErrorResponse, triggering error callbacks downstream. Example in interceptor: return next.handle(req).pipe( catchError((error: HttpErrorResponse) => { // Optional: log or handle 404 specifically without consuming if (error.status === 404) { console.log('404 intercepted but propagating'); } return throwError( => error); }) ); This pattern is standard for error-passing interceptors, as shown in official Angular docs and RxJS error handling guides.
Citations:
- 1: https://angular.dev/guide/http/interceptors
- 2: https://blog.angular-university.io/rxjs-error-handling/
- 3: https://stackoverflow.com/questions/75408985/why-the-error-message-returned-from-http-interceptor-is-ignored
- 4: https://stackoverflow.com/questions/58919538/angular8-httpinterceptor-return-value
- 5: https://stackoverflow.com/questions/65522520/catcherror-operator-showing-problem-in-angular-interceptor
- 6: https://stackoverflow.com/questions/70633922/http-error-interceptor-not-working-catcherror-not-working-with-angular-13
- 7: https://stackoverflow.com/questions/64224634/angular-9-interceptor-what-to-return-if-no-error-in-catcherror
- 8: https://medium.com/@fa.kudria/advanced-typing-and-error-interceptor-in-angular-from-theory-to-practice-265eb8d7915a
- 9: https://vocal.media/01/angular-error-handling-best-practices
- 10: https://app.studyraid.com/en/read/12449/402240/error-interceptor-implementation
🏁 Script executed:
#!/bin/bash
# Find and examine the error-handler.interceptor.ts file
fd "error-handler.interceptor.ts" src/Repository: openMF/web-app
Length of output: 104
🏁 Script executed:
#!/bin/bash
# Read the error-handler.interceptor.ts file to see the context around lines 109-114
if [ -f "src/app/core/http/error-handler.interceptor.ts" ]; then
wc -l "src/app/core/http/error-handler.interceptor.ts"
fiRepository: openMF/web-app
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Get context around lines 109-114, including imports and broader error handling
cat -n "src/app/core/http/error-handler.interceptor.ts" | head -150Repository: openMF/web-app
Length of output: 6786
Rethrow the 404 instead of emitting null to let the image service handle the error.
Returning a successful observable with null swallows the error and prevents downstream error handlers from seeing the 404. To allow the image service to decide how to handle missing client images, rethrow the error using throwError().
🔧 Suggested fix
-import { Observable } from 'rxjs';
+import { Observable, throwError } from 'rxjs';
@@
- return new Observable((observer) => {
- observer.next(null);
- observer.complete();
- });
+ return throwError(() => response);🤖 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 109 - 114, The
current branch where isClientImage404 returns a successful Observable with null
swallows the original 404; instead rethrow the original error so downstream
image service can handle it. Replace the return new Observable(...) in the
isClientImage404 branch with a rethrow using throwError(() => err) (or
throwError(() => originalError)) so the interceptor emits the error rather than
a null value; keep the isClientImage404 condition and use the same err reference
available in the interceptor's catch block.
| } else { | ||
| this.alertService.alert({ type: 'Unknown Error', message: 'Unknown Error. Please try again later.' }); | ||
| this.alertService.alert({ | ||
| type: this.translate.instant('error.unknown.type'), | ||
| message: this.translate.instant('error.unknown.message') |
There was a problem hiding this comment.
Keep the resolved backend message in the catch-all branch.
The else branch always shows the generic unknown message, so any localized defaultUserMessage / translated userMessageGlobalisationCode is thrown away for statuses like 409, 422, or 503.
🔧 Suggested fix
} else {
this.alertService.alert({
type: this.translate.instant('error.unknown.type'),
- message: this.translate.instant('error.unknown.message')
+ message: errorMessage || this.translate.instant('error.unknown.message')
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else { | |
| this.alertService.alert({ type: 'Unknown Error', message: 'Unknown Error. Please try again later.' }); | |
| this.alertService.alert({ | |
| type: this.translate.instant('error.unknown.type'), | |
| message: this.translate.instant('error.unknown.message') | |
| } else { | |
| this.alertService.alert({ | |
| type: this.translate.instant('error.unknown.type'), | |
| message: errorMessage || this.translate.instant('error.unknown.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 133 - 136, The
catch-all else branch currently discards any resolved backend message and always
uses the generic unknown message; update the else branch in the error-handling
flow (the code that calls alertService.alert and translate.instant) to prefer
the resolved backend message (e.g. the previously-derived defaultUserMessage or
the translated userMessageGlobalisationCode) when present, and only fall back to
translate.instant('error.unknown.message') if no backend message exists; ensure
you use the same translation helper (translate.instant) for the globalization
code path and pass that resolved string into alertService.alert's message field.
|
Hi @IOhacker
All CI checks are now passing. Please re-review.. |
There was a problem hiding this comment.
♻️ Duplicate comments (4)
src/app/core/http/error-handler.interceptor.ts (4)
109-114:⚠️ Potential issue | 🟠 MajorRe-throw the client-image 404 instead of converting it to
null.This branch turns the 404 into a successful emission, so downstream handlers never see an error. If the image service is supposed to decide how to handle a missing photo, it needs the original
HttpErrorResponse.In Angular HttpInterceptor with RxJS catchError, does returning an Observable that emits `null` swallow the original `HttpErrorResponse`, and should `throwError(() => error)` be used instead to keep downstream error handlers working?🔧 Suggested fix
-import { Observable } from 'rxjs'; +import { Observable, throwError } from 'rxjs'; @@ - return new Observable((observer) => { - observer.next(null); - observer.complete(); - }); + return throwError(() => response);🤖 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 109 - 114, The branch that handles isClientImage404 should re-throw the original HttpErrorResponse instead of returning an Observable that emits null; update the logic in the error handling path (in the interceptor method where isClientImage404 is checked) to return throwError(() => error) (or rethrow the error via throw) so downstream subscribers and the imaging service receive the original HttpErrorResponse rather than a successful null emission.
50-64:⚠️ Potential issue | 🟠 MajorKeep the first nested Fineract error as the winner.
Line 63 still overwrites the
errors[0]code selected on Lines 55-57. When Fineract sends both shapes, that replaces the more specific nested message with a generic top-level one and breaks the intended nested-first fallback.🔧 Suggested fix
- let errorMessage = response.error?.developerMessage || response.message; + let errorMessage = response.error?.defaultUserMessage || response.error?.developerMessage || response.message; let globalisationCode: string | null = null; @@ - if (response.error?.userMessageGlobalisationCode) { + if (!globalisationCode && response.error?.userMessageGlobalisationCode) { globalisationCode = response.error.userMessageGlobalisationCode; + errorMessage = response.error?.defaultUserMessage || errorMessage; }🤖 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 - 64, The current logic sets globalisationCode from response.error.userMessageGlobalisationCode even after selecting the nested winner from response.error.errors[0], which overwrites the more specific nested code; update the conditional that checks response.error?.userMessageGlobalisationCode so it only assigns to globalisationCode (and does not change errorMessage) if globalisationCode is still null/undefined after inspecting response.error.errors[0] — reference the variables response, globalisationCode, errorMessage and the nested response.error.errors[0] when making this change.
133-137:⚠️ Potential issue | 🟠 MajorDon't discard the resolved backend message in the catch-all branch.
Statuses like 409, 422, or 503 can already populate
errorMessageabove, but Line 136 always replaces it with the generic unknown text. That undoes the localization work for every non-enumerated status.🔧 Suggested fix
} else { this.alertService.alert({ - type: this.translate.instant('error.unknown.type'), - message: this.translate.instant('error.unknown.message') + type: this.translate.instant('errors.error.unknown.type'), + message: errorMessage || this.translate.instant('errors.error.unknown.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 133 - 137, The catch-all else branch in error-handler.interceptor.ts currently overwrites any resolved backend message by always using translate.instant('error.unknown.message'); update the else logic to only use the generic translated message when the local variable errorMessage (the resolved backend message computed earlier in the function) is empty/undefined, otherwise pass errorMessage through to this.alertService.alert; locate the branch around the this.alertService.alert call and conditionally choose between errorMessage and this.translate.instant('error.unknown.message') so backend/localized messages are preserved for statuses like 409/422/503.
66-72:⚠️ Potential issue | 🟠 MajorPrefix these HTTP error lookups with
errors..Line 68 and the status branches below are resolving keys from the root namespace, but the locale files store them under
errors(for examplesrc/assets/translations/lt-LT.json). With ngx-translate's default parser,instant('error.auth.type')misses and falls back to the key string, so alerts can render raw keys instead of localized text.Verification: confirm there is no custom parser remapping these keys, then verify that only the
errors.-prefixed paths resolve in the locale JSON.#!/bin/bash set -e # Check whether the app installs a custom ngx-translate parser / missing handler. rg -n "TranslateParser|MissingTranslationHandler|TranslateModule\\.forRoot" src/app python3 <<'PY' import json from pathlib import Path def get_value(target, key): keys = key.split(".") buf = "" cur = target while keys: buf += keys.pop(0) if isinstance(cur, dict) and buf in cur and (isinstance(cur[buf], (dict, list)) or not keys): cur = cur[buf] buf = "" elif not keys: cur = None else: buf += "." return cur data = json.loads(Path("src/assets/translations/lt-LT.json").read_text(encoding="utf-8")) for key in ( "error.auth.type", "errors.error.auth.type", "error.bad.request.type", "errors.error.bad.request.type", ): print(f"{key} => {get_value(data, key)!r}") PYExpected result: the unprefixed keys resolve to
None, while theerrors.-prefixed paths resolve to translated strings.🔧 Suggested fix pattern
- const translated = this.translate.instant(globalisationCode, response.error?.errors?.[0] || response.error || {}); - if (translated !== globalisationCode) { + const translationKey = globalisationCode.startsWith('errors.') + ? globalisationCode + : `errors.${globalisationCode}`; + const translated = this.translate.instant(translationKey, response.error?.errors?.[0] || response.error || {}); + if (translated !== translationKey) { errorMessage = translated; } @@ - type: this.translate.instant('error.auth.type'), + type: this.translate.instant('errors.error.auth.type'),Apply the same prefixing to the other hardcoded status keys as well.
Also applies to: 83-137
🤖 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 66 - 72, The translation lookups are using root keys but your locale JSON nests messages under "errors", so update all calls that build or pass translation keys to prefix them with "errors." (e.g., the translate.instant call that uses globalisationCode and every branch that constructs status-based keys in the interceptor) so that translate.instant receives "errors.<key>" instead of "<key>"; locate uses of globalisationCode, translate.instant, and the response.status handling in error-handler.interceptor.ts and prepend "errors." to those keys (and to any hardcoded keys in the 83-137 range) ensuring you don't alter other logic like fallback handling.
🤖 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 109-114: The branch that handles isClientImage404 should re-throw
the original HttpErrorResponse instead of returning an Observable that emits
null; update the logic in the error handling path (in the interceptor method
where isClientImage404 is checked) to return throwError(() => error) (or rethrow
the error via throw) so downstream subscribers and the imaging service receive
the original HttpErrorResponse rather than a successful null emission.
- Around line 50-64: The current logic sets globalisationCode from
response.error.userMessageGlobalisationCode even after selecting the nested
winner from response.error.errors[0], which overwrites the more specific nested
code; update the conditional that checks
response.error?.userMessageGlobalisationCode so it only assigns to
globalisationCode (and does not change errorMessage) if globalisationCode is
still null/undefined after inspecting response.error.errors[0] — reference the
variables response, globalisationCode, errorMessage and the nested
response.error.errors[0] when making this change.
- Around line 133-137: The catch-all else branch in error-handler.interceptor.ts
currently overwrites any resolved backend message by always using
translate.instant('error.unknown.message'); update the else logic to only use
the generic translated message when the local variable errorMessage (the
resolved backend message computed earlier in the function) is empty/undefined,
otherwise pass errorMessage through to this.alertService.alert; locate the
branch around the this.alertService.alert call and conditionally choose between
errorMessage and this.translate.instant('error.unknown.message') so
backend/localized messages are preserved for statuses like 409/422/503.
- Around line 66-72: The translation lookups are using root keys but your locale
JSON nests messages under "errors", so update all calls that build or pass
translation keys to prefix them with "errors." (e.g., the translate.instant call
that uses globalisationCode and every branch that constructs status-based keys
in the interceptor) so that translate.instant receives "errors.<key>" instead of
"<key>"; locate uses of globalisationCode, translate.instant, and the
response.status handling in error-handler.interceptor.ts and prepend "errors."
to those keys (and to any hardcoded keys in the 83-137 range) ensuring you don't
alter other logic like fallback handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20fc9573-c71c-411c-ada2-55aeee33e391
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (15)
src/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.jsonsrc/environments/.env.ts
✅ Files skipped from review due to trivial changes (5)
- src/assets/translations/es-MX.json
- src/assets/translations/de-DE.json
- src/assets/translations/cs-CS.json
- src/assets/translations/en-US.json
- src/assets/translations/fr-FR.json
🚧 Files skipped from review as they are similar to previous changes (7)
- src/assets/translations/ne-NE.json
- src/assets/translations/sw-SW.json
- src/assets/translations/pt-PT.json
- src/assets/translations/lv-LV.json
- src/assets/translations/ko-KO.json
- src/assets/translations/it-IT.json
- src/assets/translations/es-CL.json
2 participants
Description
Error messages in the Mifos web app were always displayed in English regardless of the language selected by the user. Apache Fineract sends two localisation codes in error responses — userMessageGlobalisationCode and errors.userMessageGlobalisationCode — but the error handler interceptor was completely ignoring these and always falling back to defaultUserMessage which is in English.
This PR updates the ErrorHandlerInterceptor to:
Extract userMessageGlobalisationCode from Fineract error responses — checking nested errors first (more specific), then top-level
Pass interpolation args to translate.instant() for parameterized error messages
Use errorMessage || this.translate.instant(...) pattern so server-localized messages are never discarded
Compare invalid token check against stable raw server message instead of localized text
Fix 404 client image handling — invert condition so error flows through to service handler
Use consistent .type and .message key naming throughout
Translation keys added for all HTTP error types (401, 400, 403, 404, 500, 501) across all 14 supported language files including 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.
Related issues and discussion
WEB-862: https://mifosforge.jira.com/browse/WEB-862
Summary by CodeRabbit
New Features
Chores