dify icon indicating copy to clipboard operation
dify copied to clipboard

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

Open dwgeneral opened this issue 1 year ago • 1 comments

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.

  • [x] 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:

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

dwgeneral avatar May 16 '24 08:05 dwgeneral

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?

Yeuoly avatar May 17 '24 04:05 Yeuoly