| Hi
Yes,
You should test your fix by providing valid and invalid json and check whether it was handled and proper exception was raised.
For that you can write unit test and/or integration test where you are testing one of the API and check what happens when invalid json body was provided.
Regards Adam Sent from my iPhone On 19 Mar 2024, at 02:06, Zeyad Nasef <zeyad.nasef....@gmail.com> wrote:
I don't know a specific example when it fails. All I know is that it retrieves the name and value of attributes and converts them to JSON. But I checked the stack trace shown in the issue description on Jira and saw this: org.apache.fineract.commands.service.SynchronousCommandProcessingService.publishEvent (SynchronousCommandProcessingService.java:273)
at
org.apache.fineract.commands.service.SynchronousCommandProcessingService.publishErrorEvent (SynchronousCommandProcessingService.java:236)which indicates that the exception was raised from `fromJson()` method in publishEvent (renamed to publishHookEvent). `fromJson()` throws different exceptions based on the error in the JSON string it tries to map, and all of them are wrapped up to JsonSyntaxException at the end. if `{` is missing => IllegalStateException
if `}` is missing => EOFException
if `:`, `"`, or `,` is missing => MalformedJsonException (Our case in the issue). SolutionTo address this, I've opened a PR that adds a validation method to ensure the mapping is valid here. You mentioned adding proper test cases to this fix. Were you referring to adding unit tests for the `isValidJson()` method? Let me know if we're on the same page here (or if I've missed something). We could hop on a quick call to discuss further if you're available. Thanks, Zeyad. Hi
I think you are on the right track.
Based on how deep you wanna go into the issue:
Deep:- Gson (getAsJsonObject, etc) usually throws IllegalStateException when the json is not valid or - If there is a check in the JsonHelper it might return NULL value
The 1st one can be wrapped into a more meaningful exception (like: PlatformApiDataValidationException) and Fineract can handle it with the right error code.
The 2nd one should be handled in the validators, but in some cases it might missing, those can be identified and fix to handle NULL values properly, hence avoiding NullPointerExceptions.
Not too deep, focusing on the exception handling
I dont really see where the problem is with the publishHookEvent method call during exception.
Can you please share an example where ErrorInfo cannot be mapped to a Json?
Regards, Adam
On 16 Mar 2024, at 15:55, Zeyad Nasef <zeyad.nasef....@gmail.com> wrote:
Thanks for your reply <3
First, I want to make sure I understand the issue well, so I can discuss the solutions effectively.
Here's my understanding so far (please correct me if I'm mistaken):
When the command is processing and an error occurs `publishHookEvent()` is invoked and executes the code when `(result instanceof ErrorInfo ex)` is true which tries to map the error to JSON which is not
always valid and (throw an exception).
Proposed Solution:
Implement a validation method to ensure the JSON string is valid.
What comes to my mind is something like this:
private boolean isValidJson(String json, Type type) {
try {
gson.fromJson(json, type);
return true;
} catch (Exception e) {
return false;
}
}
I couldn't understand how and why to "enhance the processor logic to handle the empty body as an empty JSON"
did you mean modify `processCommand()` in `CommandSourceService`
Thank you!
On Sat, Mar 16, 2024 at 12:04 AM Ádám Sághy <adamsa...@gmail.com> wrote:
Hi Zeyad,
I would say we should have a validation in the `SynchronousCommandProcessingService` class before executing the command to check whether the body is a valid JSON or not.
Alternatively we could enhance the processor logic to handle the empty body as an empty JSON ( {} )...
What do you think?
Regards,
Adam
> On 15 Mar 2024, at 14:47, Zeyad Nasef <zeyad.nasef....@gmail.com> wrote:
>
> Hi there.
> I encountered an issue while attempting to resolve FINERACT-1269 and I'm seeking assistance.
>
> Issue Overview:
> The problem entails a JsonSyntaxException appearing in the logs, which violates the logging guidelines outlined in the README.md, as it's an error with the client request.
>
> My Approach:
> I utilized the IntelliJ debugger to trace the code and sent a POST request to the /v1/codes endpoint. I observed that the exception arises within the JsonHelper class in fineract-core module, specifically within the parse() method, when attempting to parse invalid JSON.
>
> The Problem:
> This exception is thrown before reaching the line discussed by Michael Vorburger.
> Map<String, Object> errorMap = gson.fromJson(ex.getMessage(), type);
> Thus, idk if I misunderstood the issue. Any insights into the exact problem and suggestions on how to address it would be greatly appreciated.
> Additionally, the project's build and run times are excessively long (~10 min) each time. That's Painful :")
> I would highly appreciate any recommendations to reduce the build and run time.
>
> Apologies for the lengthy message.
> Thank you!
|
