Fix APIM gateway hang when AI metadata parsing fails (#5028)#13833
Fix APIM gateway hang when AI metadata parsing fails (#5028)#13833Tharsanan1 wants to merge 5 commits into
Conversation
Tharsanan1
requested review from
AnuGayan,
Arshardh,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
npamudika,
pubudu538,
senthuran16,
tgtshanika,
tharikaGitHub and
tharindu1st
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds upfront validation of LLM provider metadata identifiers (JSONPath or regex) in APIAdminImpl, introduces an error code, makes BuiltInLLMProviderService resilient to extraction errors by logging and returning partial metadata, adds test dependencies, and adds unit tests for service and admin validation. ChangesAI Provider Metadata Validation
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
There was a problem hiding this comment.
Pull request overview
This PR addresses a gateway hang that occurs when AI response metadata parsing fails, by making runtime metadata extraction resilient and adding configuration-time validation for LLM provider metadata identifiers.
Changes:
- Swallow/skip runtime exceptions in
BuiltInLLMProviderService.getResponseMetadata()to avoid breaking the mediation flow. - Validate configured metadata
attributeIdentifiervalues on add/update ofLLMProvider(JSONPath for payload sources, regex for path sources) and introduce a new 400 error code for invalid identifiers. - Add/extend unit tests for both runtime metadata extraction and admin-side validation, plus required test dependencies.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| components/apimgt/org.wso2.carbon.apimgt.impl/src/test/java/org/wso2/carbon/apimgt/impl/APIAdminImplTest.java | Adds unit tests covering add/update validation behavior for JSONPath/regex metadata identifiers. |
| components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java | Adds configuration-time validation of LLM provider metadata identifiers during add/update operations. |
| components/apimgt/org.wso2.carbon.apimgt.api/src/test/java/org/wso2/carbon/apimgt/api/BuiltInLLMProviderServiceTest.java | Introduces unit tests for resilient response metadata extraction (payload JSONPath + path regex). |
| components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/ExceptionCodes.java | Adds a new exception code for invalid metadata identifiers (HTTP 400). |
| components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/BuiltInLLMProviderService.java | Changes runtime metadata extraction to log-and-skip on errors rather than throwing. |
| components/apimgt/org.wso2.carbon.apimgt.api/pom.xml | Adds test dependencies needed for the new unit tests in the api module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- BuiltInLLMProviderService: catch Exception silently (log.warn) instead of re-throwing APIManagementException so the gateway never hangs on metadata extraction failure - ExceptionCodes: add AI_SERVICE_PROVIDER_INVALID_METADATA_IDENTIFIER (903104) - APIAdminImpl: validate JSONPath/regex identifiers at LLM provider save time, rejecting invalid expressions with HTTP 400 before they can reach the gateway - Tests: 13 new unit tests for BuiltInLLMProviderService; 11 new tests in APIAdminImplTest covering add/update validation paths Fixes wso2/api-manager#5028
Tharsanan1
force-pushed
the
fix/issue-5028-clean
branch
from
ddcaf9c to
0df0847
Compare
|
@coderabbitai please review |
|
✅ Actions performedReview triggered.
|
|
@coderabbitai review and approve |
|
Regarding approval — I'm only able to approve a PR when the ✅ Actions performedReview triggered.
|
|
@coderabbitai approve |
✅ Actions performedComments resolved and changes approved. |
Issue
Fixes wso2/api-manager#5028
When an LLM provider is configured with an invalid
attributeIdentifier(e.g., a regex pattern used where a JSONPath expression is expected), the gateway hangs indefinitely on every request instead of returning an error response. The root cause is thatBuiltInLLMProviderService.getResponseMetadata()throws anAPIManagementExceptionon parse failure, andAIAPIMediator.processOutboundResponse()exits before settingTARGET_ENDPOINT = EXIT_ENDPOINT. The Synapse sequence then waits for a routing signal that never arrives, causing the connection to hang until the 180-second socket timeout.Root Cause
BuiltInLLMProviderService.getResponseMetadata()wrapped bothPathNotFoundExceptionand genericExceptionin a re-thrownAPIManagementException. When this exception propagated out ofprocessOutboundResponse(), the mediator returnedfalsefrommediate()without having setTARGET_ENDPOINT = EXIT_ENDPOINT, leaving the Synapse mediation sequence in a hung state.What Was Changed
Part 1 — Gateway resilience (
BuiltInLLMProviderService.java)catch (PathNotFoundException e)re-throw block.catch (Exception e)from re-throwingAPIManagementExceptiontolog.warn(...), so the gateway always completes the response even when metadata extraction fails.Part 2 — Validation at configuration time (
APIAdminImpl.java,ExceptionCodes.java)validateLLMProviderMetadataIdentifiers()called from bothaddLLMProvider()andupdateLLMProvider().payloadinput sources: validatesattributeIdentifieras a JSONPath expression usingJsonPath.compile().pathinput sources: validatesattributeIdentifieras a regex usingPattern.compile().903104(AI_SERVICE_PROVIDER_INVALID_METADATA_IDENTIFIER).Tests
BuiltInLLMProviderServiceTest.java— 13 unit tests covering happy path, invalid JSONPath, missing fields, null inputs, regex extraction, and mixed input sources.APIAdminImplTest.java— 11 new tests covering add/update validation with valid/invalid JSONPath and regex identifiers, null config, empty identifiers, and mixed scenarios.Verification
{"code":903104,"message":"Invalid metadata identifier","description":"Invalid JSONPath expression '[0-9]+' for attribute 'promptTokenCount'"}.PathNotFoundExceptionat runtime all returned HTTP 200 in under 1 second (previously hung for 180 seconds).apimodule: 14/14,implmodule: 21/21).Test Evidence