Adam Saghy created FINERACT-2985:
------------------------------------
Summary: [BE] PL / WC - Add Recoverable Error for Retry Create
Account Failure
Key: FINERACT-2985
URL: https://issues.apache.org/jira/browse/FINERACT-2985
Project: Apache Fineract
Issue Type: Improvement
Components: System
Affects Versions: 1.16.0
Reporter: Adam Saghy
Assignee: Adam Saghy
Fix For: 1.17.0
h3. Problem
Retrying POST /clients with the same Idempotency-Key near midnight keeps
failing with error.msg.clients.activationDate.in.the.future. The activation
date is the request date, but the business-date increment job hasn’t finished
yet. The failure is saved in m_portfolio_command_source as ERROR under that
key. Every later retry with the same key replays the cached failure, even after
the business date has moved on.
h3. Approach
A failed command whose error is classified as *recoverable* is still saved with
status ERROR. In the same save, its idempotency key is renamed by adding
_RE<n>, where <n> is the number of recoverable failures so far for that key:
||Attempt||Outcome||Row saved||
|1|recoverable failure|abc_RE1, ERROR|
|2|recoverable failure|abc_RE2, ERROR|
|3|success|abc, PROCESSED|
|4|—|replayed from cache (abc, PROCESSED)|
The original key is free again, so the next retry follows the normal path:
* *No new status and no special path.* CommandProcessingResultType is
unchanged and an error is still ERROR. Nothing outside this logic needs to know
about recoverable errors.
* *No parallel execution of the same request.* Every attempt goes through
saveInitial, which inserts an UNDER_PROCESSING row under the original key. A
concurrent request with the same key hits UNIQUE_PORTFOLIO_COMMAND_SOURCE and
gets the existing 409 “under processing” response. The key is renamed only
after the attempt has finished, so at most one attempt per key runs at any time.
* *A full audit trail.* Each attempt keeps its own row with its own maker,
payload, timestamp and result. A retry never overwrites an earlier attempt, so
maker-checker always sees the user who really submitted it.
* *Non-recoverable failures behave as today.* The key isn’t renamed and the
cached failure is replayed.
h3. Which errors are recoverable
An error is recoverable if either of these is true:
# The exception class implements the marker interface
org.apache.fineract.infrastructure.core.exception.RecoverableError. This covers
errors that are recoverable by nature, decided in code.
# Its userMessageGlobalisationCode is in the global configuration
idempotency-recoverable-error-codes. This is a comma-separated string_value in
c_configuration, read the same way as the existing list-valued settings in
ConfigurationDomainServiceJpa.
** For PlatformApiDataValidationException, *every* entry in getErrors() must
be in the list. If any entry is not, the error is not recoverable.
** For other AbstractPlatformException subclasses, the exception’s own
globalisation code is checked.
** Default: enabled, with
error.msg.clients.activationDate.in.the.future,error.msg.group.activationDate.in.the.future.
The group code also covers centres.
The check lives in one component, RecoverableErrorClassifier. It looks at the
mapped exception (ErrorHandler.getMappable).
h3.
h3. Acceptance criteria
# RecoverableError.java (new): marker interface.
# RecoverableErrorClassifier.java (new): the classification rules above.
# Liquibase changeset (new):
** Adds the idempotency-recoverable-error-codes global configuration.
** Widens m_portfolio_command_source.idempotency_key from VARCHAR(50) to
VARCHAR(64), so a full-length client key still fits with the suffix. Update
@Column(length = …) in CommandSource to match.
# ConfigurationDomainService / ConfigurationDomainServiceJpa: add a getter
that returns the configured codes, or an empty set when the setting is disabled.
# CommandSourceRepository: add a count of rows with the same action_name and
entity_name whose idempotency_key matches <key>_RE%. Escape _ and % in the LIKE
pattern.
# SynchronousCommandProcessingService.persistFinalErrorResult: if the error is
recoverable, set status ERRORand idempotencyKey = key + "_RE" + (count + 1),
then save in one saveAndFlush. Nothing else in the class changes:
** exceptionWhenTheRequestAlreadyProcessed stays as it is.
** Retries inside the same request still reuse the row through
COMMAND_SOURCE_ID, because the rename happens only after the last attempt.
** Inside an enclosing batch transaction nothing is saved today, so nothing is
renamed.
** IdempotencyStoreFilter later writes the response body by row id, so the
renamed row still gets its final body.
# Collision handling: if a client used a literal key like abc_RE1 itself, the
rename can hit the unique constraint. In that case, try the next number a few
times. If that still fails, leave the key unchanged, which is today’s
behaviour. Optionally, reject incoming keys that match _RE\d+$.
h3. Tests
Unit tests:
* RecoverableErrorClassifierTest should cover:
** the marker interface → recoverable
** all validation codes configured → recoverable
** mixed codes → not recoverable
** setting disabled → not recoverable
** a domain-rule exception with a configured code → recoverable
** an unrelated exception → not recoverable
* SynchronousCommandProcessingServiceTest should cover:
** a recoverable failure saves ERROR with the key renamed to idk_RE1
** when one renamed row already exists, the new key is idk_RE2
** a non-recoverable failure keeps the key
** an enclosing transaction saves nothing
** the collision fallback
* Drop the unit tests that relied on
RECOVERABLE_ERROR(recoverableErrorStoresCommandSourceIdForRetry,
retryReusesRecoveredCommandSource).
Integration tests (ClientActivationIdempotencyRetryIntegrationTest):
* With a lagged business date the request returns 400. After the business date
moves on, a retry with the same key returns 200 and isn’t served from cache. A
third call with the same key is served from cache and returns the same clientId.
* Two recoverable failures followed by a success: both failed attempts are
still in the audit trail with their 400 results, next to the processed one. The
audit API doesn’t expose the idempotency key, so check this by action, entity
and maker.
* Two concurrent requests with the same key after a recoverable failure: one
gets 200, the other gets 409, and exactly one client is created.
* A code that isn’t in the configured list still replays the cached failure.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
