appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.57k stars 3.73k forks source link

[Bug]: Range annotation is not working for ActionConfiguration #37019

Open NilanshBansal opened 3 weeks ago

NilanshBansal commented 3 weeks ago

Is there an existing issue for this?

Description

From this code, it is expected that the timeoutInMillisecond cannot be out of the specified range but the user is able to set the range to a larger number without getting any validation errors. https://github.com/appsmithorg/appsmith/blob/203b3222c7e40d03a2c2ea9454185a4c7736fe4c/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ActionConfiguration.java#L49-L51

Steps To Reproduce

  1. Create a new query on any datasource
  2. Go to the settings tab in the query and set the Query Timeout to a very large number and click on Run, no error shows up.

image

Public Sample App

No response

Environment

Production

Severity

Medium (Frustrating UX)

Issue video log

No response

Version

v1.47

ambati-vedapriya commented 3 weeks ago

Hi @NilanshBansal ,I'm trying to reproduce on this.

ALOK9442 commented 3 weeks ago

hey @NilanshBansal , I am trying to replicate this issue, but I could not access the link that you have shared.

NilanshBansal commented 3 weeks ago

@ALOK9442 that slack link was for internal purposes, all the required information is already added to the issue 🙏

ALOK9442 commented 3 weeks ago

hey @Nikhil-Nandagopal @NilanshBansal , I would like to work on this issue, could you please assign it to me

NilanshBansal commented 3 weeks ago

@ALOK9442 please go ahead and tag @appsmithorg/query-js-pod once you have a fix PR in place.

ALOK9442 commented 3 weeks ago

@ALOK9442 please go ahead and tag @appsmithorg/query-js-pod once you have a fix PR in place.

okay I will, thanks!!

ambati-vedapriya commented 3 weeks ago

HI @NilanshBansal can you please assign this issue to me,please

ALOK9442 commented 2 weeks ago

Hey @NilanshBansal , I’m currently debugging the issue, and it seems to be related to both the backend and frontend. If we set the query limit beyond the MAX_TIMEOUT_VALUE, we should return an error message from the backend indicating that the "Query timeout" field must be an integer between MIN_TIMEOUT_VALUE and MAX_TIMEOUT_VALUE. or, we could implement frontend validation so that when a user clicks the "Run" button, they receive a prompt stating the same requirement: that the "Query timeout" field must be an integer between MIN_TIMEOUT_VALUE (0) and MAX_TIMEOUT_VALUE (60000 milliseconds). and for the frontend validation my current approach is of using the value of "timeoutInMillisecond" from the redux and we can work with it. I’d love to hear your thoughts on this approach! Thanks!

ALOK9442 commented 2 weeks ago

Hey @NilanshBansal , I’m currently debugging the issue, and it seems to be related to both the backend and frontend. If we set the query limit beyond the MAX_TIMEOUT_VALUE, we should return an error message from the backend indicating that the "Query timeout" field must be an integer between MIN_TIMEOUT_VALUE and MAX_TIMEOUT_VALUE. or, we could implement frontend validation so that when a user clicks the "Run" button, they receive a prompt stating the same requirement: that the "Query timeout" field must be an integer between MIN_TIMEOUT_VALUE (0) and MAX_TIMEOUT_VALUE (60000 milliseconds). and for the frontend validation my current approach is of using the value of "timeoutInMillisecond" from the redux and we can work with it. I’d love to hear your thoughts on this approach! Thanks!

hey @rohan-arthur could you please look into it

NilanshBansal commented 2 weeks ago

@ALOK9442 This issue is specifically created to resolve the backend, Range annotation working. Can you please check the line of code attached in the description? Here, we want the backend to validate and return the error.

ALOK9442 commented 2 weeks ago

@NilanshBansal oh!!, my bad then , also could you please un assign as I have never worked with java sorry again for quickly jumping over this issue

ambati-vedapriya commented 2 weeks ago

@NilanshBansal , Should we display the error message in the frontend as well, or is it sufficient to only log it in the terminal?

NilanshBansal commented 2 weeks ago

@ambati-vedapriya we should display the error message from the API in the frontend as well. I assume it should be auto displayed if we throw error from the backend, is it not the case?

jairoMolina9 commented 2 weeks ago

If still open, can I give this a try? Thanks!

ambati-vedapriya commented 1 week ago

Hi @NilanshBansal, after updating the code, the Range annotation is working correctly for the ActionConfiguration model. The API call is successfully blocked when the range exceeds 60,000 ms. Is there anything else that needs to be addressed? Screenshot from 2024-11-01 16-34-52 Screenshot from 2024-11-01 16-34-19 Screenshot from 2024-11-01 16-35-13

ambati-vedapriya commented 1 week ago

Hello @NilanshBansal, could you kindly make an update when you have a moment?

NilanshBansal commented 1 week ago

@ambati-vedapriya I have tried setting up a postgres datasource and configuring the timeout to a very large value. When I click on the run button the query passes and returns the result, please check the attached screen recording. The error you are seeing above is not due to timeout.

https://github.com/user-attachments/assets/db510421-7036-4288-b4ed-fd9898de2fcb