Fix stringified JSON response (#22450) (#22498)

## Description
TL;DR

This commit fixes unexpected stringified JSON in the following plugins
- REST API Plugin
- GraphQL plugin

The detailed analysis can be found in the comment section of the issue
itself (#22450)

Slack Conversations
-
[1](https://theappsmith.slack.com/archives/C0341RERY4R/p1681123872154409)
-
[2](https://theappsmith.slack.com/archives/CGBPVEJ5C/p1681726446635249)

> Add a TL;DR when description is extra long (helps content team)

Fixes #22450 


Media


## Type of change

- Bug fix (non-breaking change which fixes an issue)


## How Has This Been Tested?

- Manual

### Test Plan
> Add Testsmith test cases links that relate to this PR

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)


## Checklist:
### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
This commit is contained in:
subratadeypappu 2023-04-26 22:52:08 +06:00 committed by Ayush Pahwa
parent 93ab58536b
commit ee2ed0e951
4 changed files with 31 additions and 28 deletions

View File

@ -176,17 +176,12 @@ describe("Rest Bugs tests", function () {
);
cy.ResponseStatusCheck("404 NOT_FOUND");
cy.get(commonlocators.errorTab).should("be.visible").click({ force: true });
cy.get(commonlocators.debuggerToggle).click();
cy.wait(1000);
cy.get(commonlocators.debuggerLabel)
.invoke("text")
.then(($text) => {
expect($text).to.eq("API execution error");
});
cy.get(commonlocators.debuggerToggle).click();
cy.wait(1000);
cy.get(commonlocators.debuggerDownStreamErrCode)
.invoke("text")
.then(($text) => {
expect($text).to.eq("[404 NOT_FOUND]");
expect($text).contains("Not found");
});
});

View File

@ -281,15 +281,6 @@ public class GraphQLPlugin extends BasePlugin {
client, httpMethod, uri, requestBodyObj, actionExecutionRequest,
objectMapper, hintMessages, errorResult, requestCaptureFilter
)
.map(actionExecutionResult -> {
if (! actionExecutionResult.getIsExecutionSuccess()) {
actionExecutionResult.setErrorInfo(new AppsmithPluginException(GraphQLPluginError.QUERY_EXECUTION_FAILED,
GraphQLErrorMessages.QUERY_EXECUTION_FAILED_ERROR_MSG,
actionExecutionResult.getBody(),
actionExecutionResult.getStatusCode() ));
}
return actionExecutionResult;
})
.onErrorResume(error -> {
errorResult.setRequest(requestCaptureFilter.populateRequestFields(actionExecutionRequest));
errorResult.setIsExecutionSuccess(false);

View File

@ -182,17 +182,6 @@ public class RestApiPlugin extends BasePlugin {
client, httpMethod, uri, requestBodyObj, actionExecutionRequest,
objectMapper, hintMessages, errorResult, requestCaptureFilter
)
.map(actionExecutionResult -> {
if (! actionExecutionResult.getIsExecutionSuccess()) {
actionExecutionResult.setErrorInfo(
new AppsmithPluginException(RestApiPluginError.API_EXECUTION_FAILED,
RestApiErrorMessages.API_EXECUTION_FAILED_ERROR_MSG,
actionExecutionResult.getBody(),
actionExecutionResult.getStatusCode()
));
}
return actionExecutionResult;
})
.onErrorResume(error -> {
errorResult.setRequest(requestCaptureFilter.populateRequestFields(actionExecutionRequest));
errorResult.setIsExecutionSuccess(false);

View File

@ -1872,5 +1872,33 @@ public class RestApiPluginTest {
.filter(appErrorCode -> appErrorCode.length() != 11 || !appErrorCode.startsWith("PE-RST")).count());
}
@Test
public void whenAPIReturnsNon200_doNotStringifyResponseBody() throws IOException {
// Generate a mock response which generates a non 200 HTTP status code e.g. HTTP 500
MockWebServer mockWebServer = new MockWebServer();
MockResponse mockRedirectResponse = new MockResponse()
.setResponseCode(500)
.addHeader("Content-Type", "application/json")
.setBody("{\"statusCode\":500,\"description\":\"Internal Server Error\"}");
mockWebServer.enqueue(mockRedirectResponse);
mockWebServer.start();
HttpUrl mockHttpUrl = mockWebServer.url("/mock.codes/500");
DatasourceConfiguration dsConfig = new DatasourceConfiguration();
dsConfig.setUrl(mockHttpUrl.toString());
ActionConfiguration actionConfig = new ActionConfiguration();
actionConfig.setHttpMethod(HttpMethod.GET);
Mono<ActionExecutionResult> resultMono = pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertFalse(result.getIsExecutionSuccess());
assertTrue(result.getBody() instanceof JsonNode);
})
.verifyComplete();
}
}