Skip to content

Commit 7cb07c6

Browse files
committed
[ADD] missing-timeout: Used when a method call an external request
Calling external request needs to use timeout in order to avoid waiting for a long time You can even reproduce the case using deelay.me e.g. ```python import requests response = requests.get("https://deelay.me/5000/http://localhost:80") # It will spend 5s response = requests.get("https://deelay.me/5000/http://localhost:80", timeout=2) # timeout response = requests.get("https://deelay.me/1000/http://localhost:80", timeout=2) # fine ``` After 2s if the request doesn't have response it raises the following exception requests.exceptions.ReadTimeout: HTTPSConnectionPool(host='deelay.me', port=443): Read timed out. (read timeout=2) But if it responses <=1s it is fine Now you can test the same but using a bigger delay
1 parent 79dfe9d commit 7cb07c6

File tree

9 files changed

+272
-0
lines changed

9 files changed

+272
-0
lines changed

CONTRIBUTORS.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ contributors:
8888
* Add consider-merging-isinstance, superfluous-else-return
8989
* Fix consider-using-ternary for 'True and True and True or True' case
9090
* Add bad-docstring-quotes and docstring-first-line-empty
91+
* Add missing-timeout
9192
- Ville Skyttä <[email protected]>
9293
- Pierre-Yves David <[email protected]>
9394
- David Shea <[email protected]>: invalid sequence and slice index
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import requests
2+
3+
requests.post("http://localhost") # [missing-timeout]
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
"timeout" parameter is recommended in order to stop waiting for a call after a given number with the timeout parameter.
2+
3+
See the following requests_timeouts_ documentation:
4+
5+
- *Nearly all production code should use this parameter in nearly all requests. Failure to do so can cause your program to hang indefinitely*
6+
7+
You can reproduce the issue using deelay.me
8+
9+
.. code:: python
10+
11+
import requests
12+
response = requests.get("https://deelay.me/5000/http://localhost:80") # It will spend 5s
13+
14+
response = requests.get("https://deelay.me/5000/http://localhost:80", timeout=2) # timeout
15+
16+
response = requests.get("https://deelay.me/1000/http://localhost:80", timeout=2) # fine
17+
18+
19+
Now try to use a bigger delay value as ``https://deelay.me/1000000``
20+
21+
22+
Predefined Methods With Timeout
23+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
24+
Pylint provides set of predefined method with timeout. Those predefined
25+
methods may be changed to add new ones from Pylint configuration.
26+
27+
Following predefined methods are available:
28+
29+
* ``requests.api.get``
30+
* ``requests.api.head``
31+
* ``requests.api.options``
32+
* ``requests.api.patch``
33+
* ``requests.api.post``
34+
* ``requests.api.put``
35+
* ``requests.api.request``
36+
37+
Following option is exposed:
38+
39+
.. option:: --timeout-methods=<library.method>
40+
41+
42+
You can add new methods in your Pylint configuration
43+
using the pylint ``qname`` of the method
44+
45+
You can get it using the following code:
46+
47+
.. code:: python
48+
49+
from astroid import extract_node
50+
node = extract_node('from requests import get;get()')
51+
print(next(node.func.infer()).qname())
52+
53+
And the library needs to be installed in order to be detected
54+
55+
You can add the following possible cases ``timeout-methods``:
56+
57+
* ``http.client.HTTPConnection``
58+
* ``http.client.HTTPSConnection``
59+
* ``serial.serialcli.Serial``
60+
* ``smtplib.SMTP``
61+
* ``suds.client.Client``
62+
* ``urllib.request.urlopen``
63+
64+
.. _requests_timeouts: https://requests.readthedocs.io/en/latest/user/quickstart/#timeouts
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import requests
2+
3+
requests.post("http://localhost", timeout=10)

doc/whatsnew/2/2.15/index.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ Summary -- Release highlights
1515
New checkers
1616
============
1717

18+
Added new checker ``missing-timeout`` to warn of default timeout values that could cause
19+
a program to be hanging indefinitely.
20+
1821

1922
Removed checkers
2023
================

pylint/checkers/method_args.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
2+
# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
3+
# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt
4+
5+
"""Variables checkers for Python code."""
6+
7+
from __future__ import annotations
8+
9+
from typing import TYPE_CHECKING
10+
11+
from astroid import nodes
12+
13+
from pylint.checkers import BaseChecker, utils
14+
from pylint.interfaces import INFERENCE
15+
16+
if TYPE_CHECKING:
17+
from pylint.lint import PyLinter
18+
19+
20+
class MethodArgsChecker(BaseChecker):
21+
"""BaseChecker for method_args.
22+
23+
Checks for
24+
* missing-timeout
25+
"""
26+
27+
name = "method_args"
28+
msgs = {
29+
"W3101": (
30+
"Missing timeout argument for method '%s' can cause your program to hang indefinitely",
31+
"missing-timeout",
32+
"Used when a method needs a 'timeout' parameter "
33+
"in order to avoid waiting for a long time. If no timeout "
34+
"is specified explicitly the default value is used, for example "
35+
"for 'requests' the program will never time out (i.e. hang indefinitely). "
36+
"Notice the libraries needs to be installed and the name should be the output of pylint infered. "
37+
"from astroid import extract_node;"
38+
"node = extract_node('from requests import get;get()');"
39+
"print(next(node.func.infer()).qname())",
40+
),
41+
}
42+
options = (
43+
(
44+
"timeout-methods",
45+
{
46+
"default": (
47+
"requests.api.delete",
48+
"requests.api.get",
49+
"requests.api.head",
50+
"requests.api.options",
51+
"requests.api.patch",
52+
"requests.api.post",
53+
"requests.api.put",
54+
"requests.api.request",
55+
),
56+
"type": "csv",
57+
"metavar": "<comma separated list>",
58+
"help": "List of qualified names (i.e., library.method) which require a timeout parameter "
59+
"e.g. 'requests.api.get,requests.api.post'",
60+
},
61+
),
62+
)
63+
64+
@utils.only_required_for_messages(
65+
"missing-timeout",
66+
)
67+
def visit_call(self, node: nodes.Call) -> None:
68+
"""Check if the call needs a timeout parameter based on package.func_name
69+
configured in config.timeout_methods.
70+
71+
Package uses inferred node in order to know the package imported.
72+
"""
73+
inferred = utils.safe_infer(node.func)
74+
if inferred and inferred.qname() in self.config.timeout_methods:
75+
for keyword in node.keywords:
76+
if isinstance(keyword, nodes.Keyword) or keyword.arg == "timeout":
77+
break
78+
else:
79+
self.add_message(
80+
"missing-timeout",
81+
node=node,
82+
args=(node.func.as_string(),),
83+
confidence=INFERENCE,
84+
)
85+
86+
87+
def register(linter: PyLinter) -> None:
88+
linter.register_checker(MethodArgsChecker(linter))

requirements_test_min.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ typing-extensions~=4.2
55
pytest~=7.1
66
pytest-benchmark~=3.4
77
pytest-timeout~=2.1
8+
requests
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
"""Tests for missing-timeout."""
2+
3+
# pylint: disable=consider-using-with,import-error,no-member,no-name-in-module,reimported
4+
5+
import requests
6+
from requests import (
7+
delete,
8+
delete as delete_r,
9+
get,
10+
get as get_r,
11+
head,
12+
head as head_r,
13+
options,
14+
options as options_r,
15+
patch,
16+
patch as patch_r,
17+
post,
18+
post as post_r,
19+
put,
20+
put as put_r,
21+
request,
22+
request as request_r,
23+
)
24+
25+
# requests without timeout
26+
requests.delete("http://localhost") # [missing-timeout]
27+
requests.get("http://localhost") # [missing-timeout]
28+
requests.head("http://localhost") # [missing-timeout]
29+
requests.options("http://localhost") # [missing-timeout]
30+
requests.patch("http://localhost") # [missing-timeout]
31+
requests.post("http://localhost") # [missing-timeout]
32+
requests.put("http://localhost") # [missing-timeout]
33+
requests.request("call", "http://localhost") # [missing-timeout]
34+
35+
delete_r("http://localhost") # [missing-timeout]
36+
get_r("http://localhost") # [missing-timeout]
37+
head_r("http://localhost") # [missing-timeout]
38+
options_r("http://localhost") # [missing-timeout]
39+
patch_r("http://localhost") # [missing-timeout]
40+
post_r("http://localhost") # [missing-timeout]
41+
put_r("http://localhost") # [missing-timeout]
42+
request_r("call", "http://localhost") # [missing-timeout]
43+
44+
delete("http://localhost") # [missing-timeout]
45+
get("http://localhost") # [missing-timeout]
46+
head("http://localhost") # [missing-timeout]
47+
options("http://localhost") # [missing-timeout]
48+
patch("http://localhost") # [missing-timeout]
49+
post("http://localhost") # [missing-timeout]
50+
put("http://localhost") # [missing-timeout]
51+
request("call", "http://localhost") # [missing-timeout]
52+
53+
kwargs_wo_timeout = {}
54+
post("http://localhost", **kwargs_wo_timeout)
55+
56+
# requests valid cases
57+
requests.delete("http://localhost", timeout=10)
58+
requests.get("http://localhost", timeout=10)
59+
requests.head("http://localhost", timeout=10)
60+
requests.options("http://localhost", timeout=10)
61+
requests.patch("http://localhost", timeout=10)
62+
requests.post("http://localhost", timeout=10)
63+
requests.put("http://localhost", timeout=10)
64+
requests.request("call", "http://localhost", timeout=10)
65+
66+
delete_r("http://localhost", timeout=10)
67+
get_r("http://localhost", timeout=10)
68+
head_r("http://localhost", timeout=10)
69+
options_r("http://localhost", timeout=10)
70+
patch_r("http://localhost", timeout=10)
71+
post_r("http://localhost", timeout=10)
72+
put_r("http://localhost", timeout=10)
73+
request_r("call", "http://localhost", timeout=10)
74+
75+
delete("http://localhost", timeout=10)
76+
get("http://localhost", timeout=10)
77+
head("http://localhost", timeout=10)
78+
options("http://localhost", timeout=10)
79+
patch("http://localhost", timeout=10)
80+
post("http://localhost", timeout=10)
81+
put("http://localhost", timeout=10)
82+
request("call", "http://localhost", timeout=10)
83+
84+
kwargs_timeout = {'timeout': 10}
85+
post("http://localhost", **kwargs_timeout)
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
missing-timeout:26:0:26:35::Missing timeout argument for method 'requests.delete' can cause your program to hang indefinitely:INFERENCE
2+
missing-timeout:27:0:27:32::Missing timeout argument for method 'requests.get' can cause your program to hang indefinitely:INFERENCE
3+
missing-timeout:28:0:28:33::Missing timeout argument for method 'requests.head' can cause your program to hang indefinitely:INFERENCE
4+
missing-timeout:29:0:29:36::Missing timeout argument for method 'requests.options' can cause your program to hang indefinitely:INFERENCE
5+
missing-timeout:30:0:30:34::Missing timeout argument for method 'requests.patch' can cause your program to hang indefinitely:INFERENCE
6+
missing-timeout:31:0:31:33::Missing timeout argument for method 'requests.post' can cause your program to hang indefinitely:INFERENCE
7+
missing-timeout:32:0:32:32::Missing timeout argument for method 'requests.put' can cause your program to hang indefinitely:INFERENCE
8+
missing-timeout:33:0:33:44::Missing timeout argument for method 'requests.request' can cause your program to hang indefinitely:INFERENCE
9+
missing-timeout:35:0:35:28::Missing timeout argument for method 'delete_r' can cause your program to hang indefinitely:INFERENCE
10+
missing-timeout:36:0:36:25::Missing timeout argument for method 'get_r' can cause your program to hang indefinitely:INFERENCE
11+
missing-timeout:37:0:37:26::Missing timeout argument for method 'head_r' can cause your program to hang indefinitely:INFERENCE
12+
missing-timeout:38:0:38:29::Missing timeout argument for method 'options_r' can cause your program to hang indefinitely:INFERENCE
13+
missing-timeout:39:0:39:27::Missing timeout argument for method 'patch_r' can cause your program to hang indefinitely:INFERENCE
14+
missing-timeout:40:0:40:26::Missing timeout argument for method 'post_r' can cause your program to hang indefinitely:INFERENCE
15+
missing-timeout:41:0:41:25::Missing timeout argument for method 'put_r' can cause your program to hang indefinitely:INFERENCE
16+
missing-timeout:42:0:42:37::Missing timeout argument for method 'request_r' can cause your program to hang indefinitely:INFERENCE
17+
missing-timeout:44:0:44:26::Missing timeout argument for method 'delete' can cause your program to hang indefinitely:INFERENCE
18+
missing-timeout:45:0:45:23::Missing timeout argument for method 'get' can cause your program to hang indefinitely:INFERENCE
19+
missing-timeout:46:0:46:24::Missing timeout argument for method 'head' can cause your program to hang indefinitely:INFERENCE
20+
missing-timeout:47:0:47:27::Missing timeout argument for method 'options' can cause your program to hang indefinitely:INFERENCE
21+
missing-timeout:48:0:48:25::Missing timeout argument for method 'patch' can cause your program to hang indefinitely:INFERENCE
22+
missing-timeout:49:0:49:24::Missing timeout argument for method 'post' can cause your program to hang indefinitely:INFERENCE
23+
missing-timeout:50:0:50:23::Missing timeout argument for method 'put' can cause your program to hang indefinitely:INFERENCE
24+
missing-timeout:51:0:51:35::Missing timeout argument for method 'request' can cause your program to hang indefinitely:INFERENCE

0 commit comments

Comments
 (0)