Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: the delete method is not supported the write_timeout params in requests library #4451

Conversation

dwgeneral
Copy link

Description

in v0.6.7, we introduced the timeout params for HTTP node in workflow, but I found the bug that if we call a request with DELETE method, then it will occur the invalid timeout params error, because the write_timeout was not supported for DELETE method in requests library, that's why we got this issue.

Fixes # (issue)
The reason why the DELETE method in Python’s requests library supports connect_timeout and read_timeout but not write_timeout is related to the nature of the HTTP DELETE method and how it typically behaves in web applications.

1.	Connect Timeout: This timeout refers to the time it takes for the client to establish a connection with the server. It’s relevant for all HTTP methods, including DELETE, as establishing a connection is fundamental to any HTTP request.
2.	Read Timeout: This timeout refers to the time allowed for the server to send a response back to the client after the connection is established. Again, this timeout is relevant for all HTTP methods, including DELETE, as the client expects a response from the server.
3.	Write Timeout: This timeout refers to the time allowed for the client to send the entire request (including the request body, if any) to the server. It’s more relevant for HTTP methods like POST and PUT, where the client is sending data to the server.

Type of Change

Please delete options that are not relevant.

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

How Has This Been Tested?

re-deploy the docker image and run, the issue was gone.

Suggested Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🐞 bug Something isn't working labels May 16, 2024
@dwgeneral dwgeneral changed the title fix: the delete method is not supported the write_timeout params in request lib fix: the delete method is not supported the write_timeout params in requests library May 16, 2024
@takatost takatost requested a review from Yeuoly May 16, 2024 08:51
@Yeuoly
Copy link
Collaborator

Yeuoly commented May 17, 2024

hi, thanks for your contributions! but this issue has already been solved inside ssrf_proxy, we have removed the third timeout param in timeout tuple, eliminating the need for special handling for the delete method like if xx == delete, BTW, the comments are not very clear, would you mind add some comments to make it more readable and less confusing?

@dwgeneral dwgeneral closed this May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants