fix(api): reduce ERROR-log noise from expected client failures#2
Open
Danswar wants to merge 1 commit into
Open
fix(api): reduce ERROR-log noise from expected client failures#2Danswar wants to merge 1 commit into
Danswar wants to merge 1 commit into
Conversation
- realunit: an upstream Aktionariat 4xx (e.g. business 400) was wrapped in a 503 ServiceUnavailableException and self-logged at ERROR, producing two ERROR lines per occurrence. Log at WARN and surface upstream 4xx as BadRequestException so it is not counted as a server fault. - buy-crypto: the expected ConflictException (duplicate KYC file ID) raised during the AML check is now logged at WARN instead of ERROR; all other failures stay ERROR.
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.
Goal
Reduce recurring ERROR-level log noise in dfx-api from two cases that are actually expected client/business outcomes, not server faults. Together these account for ~50+ ERROR lines/day in prod and pollute error dashboards/alerts.
Changes
1. realunit — Aktionariat upstream 4xx (
realunit.service.ts)When
requestPaymentInstructionsfailed, an upstream client/business 400 (e.g."User must have…") was:ERROR, andServiceUnavailableException(503) → the globalApiExceptionFilter(status >= 500) logged it atERRORagain.So one upstream 4xx produced two ERROR lines and was misclassified as a server outage.
Fix: log at
WARN, and when the upstream status is a 4xx, surface it asBadRequestException(4xx) instead of 503. Upstream 5xx / network errors still throwServiceUnavailableException./v1/realunit/buy/:id/confirm. The response body message is preserved.2. buy-crypto — expected duplicate-KYC ConflictException (
buy-crypto-preparation.service.ts)doAmlCheck()logsError during buy-crypto <id> AML checkatERROR. The common case is aConflictException: A user with this KYC file ID already exists— an expected guard (the AML path allocateskycFileId = MAX+1, which races against the unique constraint). It's already handled (the loop continues); only the log level was wrong. This is also the source of the matchingduplicate key value violates unique constraintlines in api-postgres.Fix: log
ConflictExceptionatWARN; all other exceptions stayERROR. Follows the existinginstanceof ConflictExceptionidiom used inbank-tx.service.ts,liquidity-management.service.ts, etc. No AML/business behavior change — the kycFileId is simply not reassigned this run, exactly as today.Validation
prettier --check✅eslint --no-fix✅nest build✅jest realunit buy-crypto-preparation tracing→ 6 suites / 77 tests pass ✅Not addressed (intentionally)
The bulk of dfx-api ERRORs are genuine external-dependency failures (blockchain RPC flaps, CoinGecko ETIMEDOUT, Olky 403) that the team monitors — those are root-cause/infra items, not log-level noise, so left untouched.