Fixing sending common error messages and 500 error code for all errors from consent core service - #992
Open
Ashi1993 wants to merge 1 commit into
Open
Fixing sending common error messages and 500 error code for all errors from consent core service#992Ashi1993 wants to merge 1 commit into
Ashi1993 wants to merge 1 commit into
Conversation
…s from consent core service
📝 WalkthroughSummary
WalkthroughThe change introduces 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java (1)
1426-1431: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStandardize this exception and preserve the cause.
This handler uses the single-argument
ConsentManagementExceptionconstructor. It omits aConsentMgtErrorCodesvalue and drops the causee, soConsentExtensionUtilscannot map a response status and the original stack trace is lost. Lines 1530-1532 have the same gap.Use
ConsentMgtErrorCodes.INTERNAL_ERRORand passeas the cause at both locations.♻️ Proposed fix
} catch (IdentityOAuth2Exception e) { log.error(String.format("Error while revoking tokens for the consent ID: %s", consentID.replaceAll("[\r\n]", "")), e); - throw new ConsentManagementException("Error occurred while revoking tokens for the consent ID: " - + consentID); + throw new ConsentManagementException(ConsentMgtErrorCodes.INTERNAL_ERROR, + "Error occurred while revoking tokens for the consent", e); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java` around lines 1426 - 1431, Update both IdentityOAuth2Exception handlers in ConsentCoreServiceImpl, including the handler around the token-revocation log and the corresponding block near the other reported location, to construct ConsentManagementException with ConsentMgtErrorCodes.INTERNAL_ERROR and the caught exception e as its cause. Preserve the existing error message and logging behavior.
🧹 Nitpick comments (3)
financial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/exception/ConsentManagementException.java (1)
40-48: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider normalizing a null
errorCodetoINTERNAL_ERROR.The explicit-code constructors accept any value, including
null.ConsentExtensionUtils.mapToResponseStatusswitches on this value, so anullcode causes a failure inside the error-handling path. Current in-repo callers pass enum literals, so this is defensive hardening only.♻️ Proposed hardening
public ConsentManagementException(ErrorConstants.ConsentMgtErrorCodes errorCode, String message) { super(message); - this.errorCode = errorCode; + this.errorCode = errorCode != null ? errorCode : ErrorConstants.ConsentMgtErrorCodes.INTERNAL_ERROR; } public ConsentManagementException(ErrorConstants.ConsentMgtErrorCodes errorCode, String message, Throwable e) { super(message, e); - this.errorCode = errorCode; + this.errorCode = errorCode != null ? errorCode : ErrorConstants.ConsentMgtErrorCodes.INTERNAL_ERROR; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@financial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/exception/ConsentManagementException.java` around lines 40 - 48, In the ConsentManagementException constructor that accepts ErrorConstants.ConsentMgtErrorCodes errorCode, normalize a null errorCode parameter to ErrorConstants.ConsentMgtErrorCodes.INTERNAL_ERROR before assigning it to this.errorCode. Apply this defensive check to both the two-parameter constructor and the three-parameter constructor to ensure mapToResponseStatus never receives a null code value.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/common/ConsentExtensionUtils.java (1)
206-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding an operation-aware overload that also preserves the cause.
toConsentExceptionWithCausedoes not accept aConsentOperationEnum. Several admin handlers call it for operations that now have dedicated enum values, for exampleCONSENT_FILE_SEARCH,CONSENT_ATTRIBUTES_SEARCH, andCONSENT_AMENDMENT_HISTORY_RETRIEVAL. Those call sites lose the operation metadata that the rest of this change adds.An overload that takes both the operation and the cause would let those handlers keep both. This requires a matching
ConsentExceptionconstructor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/common/ConsentExtensionUtils.java` around lines 206 - 208, Add an overloaded version of the toConsentExceptionWithCause method that accepts both a ConsentOperationEnum parameter and the ConsentManagementException parameter, preserving the operation metadata alongside the cause. Ensure the ConsentException class has a matching constructor that accepts both the operation enum and the cause exception so the overload can pass both values when creating the ConsentException. Keep the existing single-parameter toConsentExceptionWithCause method unchanged for backward compatibility.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/test/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentMgtCoreServiceTests.java (1)
3315-3340: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the
ConsentMgtErrorCodesvalue in consent-not-found tests.
ConsentManagementException.getErrorCode()provides the stored code. Replace theexpectedExceptionsassertions with explicittry/catchassertions so these tests cover the intended classification: useBAD_REQUESTfortestGetConsentNotFoundandNOT_FOUNDfortestGetConsentAttributesConsentNotFound.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/test/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentMgtCoreServiceTests.java` around lines 3315 - 3340, The consent-not-found tests only assert exception type and must also verify error classification. Update testGetConsentNotFound and testGetConsentAttributesConsentNotFound to explicitly catch ConsentManagementException, assert getErrorCode() matches ConsentMgtErrorCodes.BAD_REQUEST and ConsentMgtErrorCodes.NOT_FOUND respectively, and fail if no exception is thrown; remove their expectedExceptions annotations. Leave testGetConsentAttributesWithAttributeKeysConsentNotFound unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/admin/impl/DefaultConsentAdminHandler.java`:
- Line 333: Update ConsentExtensionUtils conversion usage in
DefaultConsentAdminHandler at
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/admin/impl/DefaultConsentAdminHandler.java
lines 333, 353, 410, 421, 455, and 514 to use one operation-aware helper that
preserves the caught ConsentManagementException as the cause and includes each
enclosing operation. Apply the equivalent change in DefaultConsentPersistStep at
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/impl/DefaultConsentPersistStep.java
line 112, including the persistence operation and retaining the caught cause.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java`:
- Around line 1059-1065: Retrieval-exception handlers in ConsentCoreServiceImpl
must roll back pending mutations before classifying and throwing. In
ConsentCoreServiceImpl lines 1059-1065 (updateConsentStatus), 1413-1417
(revokeConsentWithReason), and 1679-1683
(reAuthorizeConsentWithNewAuthResource), call
DatabaseUtils.rollbackTransaction(connection) at the start of each
ConsentDataRetrievalException handler; in lines 1947-1952
(updateConsentAttributes), add the rollback call to both retrieval-exception
handlers before their existing classification logic.
- Around line 213-219: Update the ConsentDataRetrievalException handling in
ConsentCoreServiceImpl to map NO_RECORDS_FOUND_ERROR_MSG to
ConsentMgtErrorCodes.NOT_FOUND instead of BAD_REQUEST, while preserving
INTERNAL_ERROR for other retrieval failures and the existing exception details.
---
Outside diff comments:
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java`:
- Around line 1426-1431: Update both IdentityOAuth2Exception handlers in
ConsentCoreServiceImpl, including the handler around the token-revocation log
and the corresponding block near the other reported location, to construct
ConsentManagementException with ConsentMgtErrorCodes.INTERNAL_ERROR and the
caught exception e as its cause. Preserve the existing error message and logging
behavior.
---
Nitpick comments:
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/exception/ConsentManagementException.java`:
- Around line 40-48: In the ConsentManagementException constructor that accepts
ErrorConstants.ConsentMgtErrorCodes errorCode, normalize a null errorCode
parameter to ErrorConstants.ConsentMgtErrorCodes.INTERNAL_ERROR before assigning
it to this.errorCode. Apply this defensive check to both the two-parameter
constructor and the three-parameter constructor to ensure mapToResponseStatus
never receives a null code value.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/common/ConsentExtensionUtils.java`:
- Around line 206-208: Add an overloaded version of the
toConsentExceptionWithCause method that accepts both a ConsentOperationEnum
parameter and the ConsentManagementException parameter, preserving the operation
metadata alongside the cause. Ensure the ConsentException class has a matching
constructor that accepts both the operation enum and the cause exception so the
overload can pass both values when creating the ConsentException. Keep the
existing single-parameter toConsentExceptionWithCause method unchanged for
backward compatibility.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/test/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentMgtCoreServiceTests.java`:
- Around line 3315-3340: The consent-not-found tests only assert exception type
and must also verify error classification. Update testGetConsentNotFound and
testGetConsentAttributesConsentNotFound to explicitly catch
ConsentManagementException, assert getErrorCode() matches
ConsentMgtErrorCodes.BAD_REQUEST and ConsentMgtErrorCodes.NOT_FOUND
respectively, and fail if no exception is thrown; remove their
expectedExceptions annotations. Leave
testGetConsentAttributesWithAttributeKeysConsentNotFound unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ccd6f5d-efa6-4e14-85e5-e378f3f5b904
📒 Files selected for processing (11)
financial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/constant/ErrorConstants.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/exception/ConsentManagementException.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/admin/impl/DefaultConsentAdminHandler.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/impl/DefaultConsentPersistStep.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/impl/DefaultConsentRetrievalStep.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/common/ConsentExtensionUtils.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/common/ConsentOperationEnum.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/common/idempotency/IdempotencyValidator.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/manage/impl/DefaultConsentManageHandler.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.javafinancial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/test/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentMgtCoreServiceTests.java
| } catch (ConsentManagementException e) { | ||
| log.error("Error while retrieving consent amendment history data", e); | ||
| throw new ConsentException(ResponseStatus.INTERNAL_SERVER_ERROR, e.getMessage(), e); | ||
| throw ConsentExtensionUtils.toConsentExceptionWithCause(e); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use one operation-aware, cause-preserving conversion.
The selected overloads do not carry the full error context required by these paths. Add or use one helper that preserves the original ConsentManagementException and the operation.
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org.wso2/financial/services/accelerator/consent/mgt/extensions/admin/impl/DefaultConsentAdminHandler.java#L333-L333: include the enclosing operation; apply the same change at Lines 353, 410, 421, 455, and 514.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org.wso2/financial/services/accelerator/consent/mgt/extensions/authorize/impl/DefaultConsentPersistStep.java#L112-L112: include the persistence operation and retain the caught cause.
📍 Affects 2 files
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/admin/impl/DefaultConsentAdminHandler.java#L333-L333(this comment)financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/impl/DefaultConsentPersistStep.java#L112-L112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/admin/impl/DefaultConsentAdminHandler.java`
at line 333, Update ConsentExtensionUtils conversion usage in
DefaultConsentAdminHandler at
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/admin/impl/DefaultConsentAdminHandler.java
lines 333, 353, 410, 421, 455, and 514 to use one operation-aware helper that
preserves the caught ConsentManagementException as the cause and includes each
enclosing operation. Apply the equivalent change in DefaultConsentPersistStep at
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/impl/DefaultConsentPersistStep.java
line 112, including the persistence operation and retaining the caught cause.
Comment on lines
213
to
+219
| } catch (ConsentDataRetrievalException e) { | ||
| log.error(ConsentCoreServiceConstants.DATA_RETRIEVE_ERROR_MSG, e); | ||
| throw new ConsentManagementException(ConsentCoreServiceConstants.DATA_RETRIEVE_ERROR_MSG, e); | ||
| if (ConsentMgtDAOConstants.NO_RECORDS_FOUND_ERROR_MSG.equals(e.getMessage())) { | ||
| throw new ConsentManagementException(ConsentMgtErrorCodes.BAD_REQUEST, e.getMessage(), e); | ||
| } else { | ||
| throw new ConsentManagementException(ConsentMgtErrorCodes.INTERNAL_ERROR, e.getMessage(), e); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify how NO_RECORDS_FOUND_ERROR_MSG is produced in the DAO layer.
set -euo pipefail
# Locate the constant definition.
rg -n --type=java -C3 'NO_RECORDS_FOUND_ERROR_MSG'
# Inspect ConsentDataRetrievalException construction sites for wrapped or formatted messages.
rg -n --type=java -C4 'new ConsentDataRetrievalException'Repository: wso2/financial-services-accelerator
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files sample:"
git ls-files | head -50
echo
echo "Find target file:"
fd -a 'ConsentCoreServiceImpl.java|ConsentMgtDAOConstants.java|ConsentDataRetrievalException.java|ConsentMgtErrorCodes.java|ConsentExtensionUtils.java' . | sed 's#^\./##'
echo
echo "Search target identifiers (case-sensitive):"
rg -n -C2 'NO_RECORDS_FOUND_ERROR_MSG|ConsentDataRetrievalException|ConsentMgtErrorCodes\.NOT_FOUND|ConsentMgtErrorCodes\.BAD_REQUEST|ConsentExtensionUtils' . || trueRepository: wso2/financial-services-accelerator
Length of output: 4619
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search identifier (ignore path type filtering):"
rg -n -C2 'NO_RECORDS_FOUND_ERROR_MSG|ConsentDataRetrievalException|ConsentMgtErrorCodes\.NOT_FOUND|ConsentMgtErrorCodes\.BAD_REQUEST|ConsentExtensionUtils|ConsentCoreServiceImpl.java|ConsentMgtDAOConstants.java' . || true
echo
echo "Focus target service file with head/sed:"
target='financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java'
if [ -f "$target" ]; then
wc -l "$target"
sed -n '170,225p' "$target" | cat -n
sed -n '1745,1820p' "$target" | cat -n
else
fd -a 'ConsentCoreServiceImpl.java' . || true
fiRepository: wso2/financial-services-accelerator
Length of output: 50392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked Java files count:"
git ls-files '*.java' | wc -l
echo
echo "Matches for DAO constant and exception construction (limited):"
rg -n -C2 --type=java 'NO_RECORDS_FOUND_ERROR_MSG' . | head -80 || true
echo
echo "Matches for ConsentDataRetrievalException construction (limited):"
rg -n -C3 --type=java 'new ConsentDataRetrievalException' . | head -160 || true
echo
echo "Target service file relevant line counts:"
target='financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java'
python3 - <<'PY' "$target"
import sys, pathlib
path = pathlib.Path(sys.argv[1])
text = path.read_text()
lines = text.splitlines()
for needle, start, end in [
('BAD_REQUEST in NO_RECORDS_FOUND_ERROR_MSG handlers', 183, 220),
('NOT_FOUND resource checks', 1745, 1815),
]:
print(f"{needle}:")
for i in range(start, min(end, len(lines)) + 1):
if lines[i-1]:
print(f"{i:4d}: {lines[i-1]}")
PYRepository: wso2/financial-services-accelerator
Length of output: 50391
Align the missing-record path to NOT_FOUND.
ConsentDataRetrievalException(NO_RECORDS_FOUND_ERROR_MSG) is used to express that a consent resource is absent, but this handler converts it to BAD_REQUEST while direct absence checks use NOT_FOUND. Map the same DAO condition consistently to one missing-record error code so API consumers receive the same HTTP status for the same condition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java`
around lines 213 - 219, Update the ConsentDataRetrievalException handling in
ConsentCoreServiceImpl to map NO_RECORDS_FOUND_ERROR_MSG to
ConsentMgtErrorCodes.NOT_FOUND instead of BAD_REQUEST, while preserving
INTERNAL_ERROR for other retrieval failures and the existing exception details.
Comment on lines
1059
to
+1065
| } catch (ConsentDataRetrievalException e) { | ||
| log.error(ConsentCoreServiceConstants.DATA_RETRIEVE_ERROR_MSG, e); | ||
| throw new ConsentManagementException(ConsentCoreServiceConstants.DATA_RETRIEVE_ERROR_MSG, e); | ||
| if (ConsentMgtDAOConstants.NO_RECORDS_FOUND_ERROR_MSG.equals(e.getMessage())) { | ||
| throw new ConsentManagementException(ConsentMgtErrorCodes.BAD_REQUEST, e.getMessage(), e); | ||
| } else { | ||
| throw new ConsentManagementException(ConsentMgtErrorCodes.INTERNAL_ERROR, e.getMessage(), e); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Roll back before classifying a retrieval failure that follows a mutation.
Four ConsentDataRetrievalException handlers classify the error and throw while a mutation from the same transaction is still pending. None of them calls DatabaseUtils.rollbackTransaction(connection). The handlers for ConsentDataInsertionException and ConsentDataUpdationException in the same methods do call it, and updateAuthorizationStatus (lines 804-811) and updateAuthorizationUser (lines 852-859) apply the same treatment to their retrieval handlers. Add the rollback call at each site below.
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1059-L1065: inupdateConsentStatus, roll back before classifying;updateConsentStatusat line 1037 already mutated data.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1413-L1417: inrevokeConsentWithReason, roll back before classifying;updateConsentStatusat line 1340 andupdateConsentMappingStatusat line 1386 already mutated data.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1679-L1683: inreAuthorizeConsentWithNewAuthResource, roll back before classifying; the authorization and mapping updates at lines 1634-1652 already mutated data.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1947-L1952: inupdateConsentAttributes, roll back in both handlers;updateConsentAttributesat line 1940 already mutated data.
📍 Affects 1 file
financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1059-L1065(this comment)financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1413-L1417financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1679-L1683financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java#L1947-L1952
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/impl/ConsentCoreServiceImpl.java`
around lines 1059 - 1065, Retrieval-exception handlers in ConsentCoreServiceImpl
must roll back pending mutations before classifying and throwing. In
ConsentCoreServiceImpl lines 1059-1065 (updateConsentStatus), 1413-1417
(revokeConsentWithReason), and 1679-1683
(reAuthorizeConsentWithNewAuthResource), call
DatabaseUtils.rollbackTransaction(connection) at the start of each
ConsentDataRetrievalException handler; in lines 1947-1952
(updateConsentAttributes), add the rollback call to both retrieval-exception
handlers before their existing classification logic.
aka4rKO
approved these changes
Aug 5, 2026
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
2 participants
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.
Fixing sending common error messages and 500 error code for all errors from consent core service
Issue link: #991
Doc Issue: Optional, link issue from documentation repository
Applicable Labels: Spec, product, version, type (specify requested labels)
Development Checklist
Testing Checklist