From a2127ec3d37006bee93096980af1553de29ee3ff Mon Sep 17 00:00:00 2001 From: tkAppsmith <131347120+tkAppsmith@users.noreply.github.com> Date: Thu, 25 May 2023 11:30:54 +0530 Subject: [PATCH] fix: Changed error message to be thrown on failed parsing of executeActionDTO (#23321) ## Description > A malformed body (multipart form) for the /execute endpoint leads to an error on UI showing: "invalid parameter: executeActionDTO". > This has now been changed in this pr to show error as: "The request could not be understood by the server due to malformed syntax." #### PR fixes following issue(s) Fixes #23250 (partly) #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] Jest - [ ] Cypress > > #### Test Plan > This can be tested by calling /execute endpoint with malformed form data since replicating this issue using UI is unreliable. The UI in general doesn't produce a malformed request body. ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Test-plan-implementation#speedbreaker-features-to-consider-for-every-change) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans/_edit#areas-of-interest) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../java/com/appsmith/server/exceptions/AppsmithError.java | 1 + .../java/com/appsmith/server/exceptions/AppsmithErrorCode.java | 1 + .../server/solutions/ce/ActionExecutionSolutionCEImpl.java | 2 +- .../server/solutions/ce/ActionExecutionSolutionCEImplTest.java | 3 +-- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java index efce40df42..e5cc646708 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java @@ -82,6 +82,7 @@ public enum AppsmithError { ACL_NO_RESOURCE_FOUND(404, AppsmithErrorCode.ACL_NO_RESOURCE_FOUND.getCode(), "Unable to find {0} {1}. Either the asset doesn''t exist or you don''t have required permissions", AppsmithErrorAction.DEFAULT, "No resource found or permission denied", ErrorType.INTERNAL_ERROR, null), GENERIC_BAD_REQUEST(400, AppsmithErrorCode.GENERIC_BAD_REQUEST.getCode(), "Bad request: {0}", AppsmithErrorAction.DEFAULT, "Invalid request", ErrorType.BAD_REQUEST, null), + GENERIC_REQUEST_BODY_PARSE_ERROR(400, AppsmithErrorCode.MALFORMED_REQUEST.getCode(), "Server cannot understand the request, malformed payload. Contact support for help.", AppsmithErrorAction.DEFAULT, "Malformed request body", ErrorType.BAD_REQUEST, null), VALIDATION_FAILURE(400, AppsmithErrorCode.VALIDATION_FAILURE.getCode(), "Validation Failure(s): {0}", AppsmithErrorAction.DEFAULT, "Validation failed", ErrorType.INTERNAL_ERROR, null), INVALID_CURL_COMMAND(400, AppsmithErrorCode.INVALID_CURL_COMMAND.getCode(), "Invalid cURL command, couldn''t import.", AppsmithErrorAction.DEFAULT, "Invalid cURL command", ErrorType.ARGUMENT_ERROR, null), INVALID_LOGIN_METHOD(401, AppsmithErrorCode.INVALID_LOGIN_METHOD.getCode(), "Please use {0} authentication to login to Appsmith", AppsmithErrorAction.DEFAULT, "Invalid login method", ErrorType.INTERNAL_ERROR, null), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java index 90590ea220..e95a58be9c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java @@ -64,6 +64,7 @@ public enum AppsmithErrorCode { GOOGLE_RECAPTCHA_TIMEOUT("AE-APP-5042", "Google recaptcha timeout"), NAME_CLASH_NOT_ALLOWED_IN_REFACTOR("AE-AST-4009", "Name clash not allowed in refactor"), GENERIC_BAD_REQUEST("AE-BAD-4000", "Generic bad request"), + MALFORMED_REQUEST("AE-BAD-4001", "Malformed request body"), GOOGLE_RECAPTCHA_FAILED("AE-CAP-4035", "Google recaptcha failed"), INVALID_CRUD_PAGE_REQUEST("AE-CRD-4039", "Invalid crud page request"), EMPTY_CURL_INPUT_STATEMENT("AE-CRL-4054", "Invalid CURL input statement"), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java index c9c40fc161..e97fc6733e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java @@ -306,7 +306,7 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE return Mono.just(objectMapper.readValue(byteData, ExecuteActionDTO.class)); } catch (IOException e) { log.error("Error in deserializing ExecuteActionDTO", e); - return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, EXECUTE_ACTION_DTO)); + return Mono.error(new AppsmithException(AppsmithError.GENERIC_REQUEST_BODY_PARSE_ERROR)); } }) .flatMap(executeActionDTO -> { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImplTest.java index 8b555c419e..178385a7f5 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImplTest.java @@ -205,7 +205,7 @@ class ActionExecutionSolutionCEImplTest { StepVerifier .create(actionExecutionResultMono) .expectErrorMatches(e -> e instanceof AppsmithException && - e.getMessage().equals(AppsmithError.INVALID_PARAMETER.getMessage("executeActionDTO"))) + e.getMessage().equals(AppsmithError.GENERIC_REQUEST_BODY_PARSE_ERROR.getMessage())) .verify(); } @@ -407,5 +407,4 @@ class ActionExecutionSolutionCEImplTest { }) .verifyComplete(); } - } \ No newline at end of file