In most cases, HTTP requests are sent to external systems. These systems as well as the underlying communication network are out of control of the application developers. They must expect that the request can hang, otherwise they risk that the current application or thread is being blocked indefinitely. This problem can be addressed by specifying a timeout for the request.

Some HTTP client libraries, such as aiohttp (opens new window) or HTTPX (opens new window) use default values for request timeouts. However, the very popular requests (opens new window) library does not have a default request timeout as of v2.24.

What if we had a tool that could automatically detect missing timeouts or even add a default value?

An overview of static analysis tools in Python

Detecting suspicious code requires some way of analyzing the source code of an application. There are the linting tools flake8 (opens new window) and pylint (opens new window) which perform code analysis and report violations based on a set of predefined rules. Both tools support custom plugins, so we could write a plugin that checks for HTTP requests without timeouts and reports them. In fact, there is an existing pylint plugin that does exactly that. It's called pylint-requests (opens new window).

Most teams will introduce a "no linting errors" policy along with one of these tools. Success is not guaranteed, though: I have seen code bases that contained # pylint or # noqa comments all over the place just to silence the linting tool instead of addressing the program code. The preferred approach would be to configure the linter correctly or simply fix the issues found by the linter. However, when static analysis tools are introduced late in a project, it may not be feasible to address all warnings in reasonable time. This leads to the development team being flooded with lint warnings, which in turn causes the team to ignore the warnings altogether.

Enter libCST (opens new window). LibCST is a static code analysis tool that supports both finding problematic code and providing an automatic fix for them. Flake8 and Pylint use the ast (opens new window) or astroid (opens new window) modules for analyzing the source code, both of which use an abstract syntax tree (AST) for code representation. The abstract syntax tree no longer contains information about the formatting of the code, such as whether a string was surrounded by single quotes or double quotes. This poses a challenge when an AST should be converted back to source code. LibCST on the other hand is backed by a concrete syntax tree, hence the name. A concrete syntax tree contains all tokens, including quotes and whitespaces. This makes it much easier for libCST to apply code transformations and preserve the formatting in the process.

Back to our task: We want to make sure that every HTTP call has a timeout. LibCST is well capable of solving this on its own. However, there is quite a bit of boilerplate code needed for each linting rule. This is why Fixit (opens new window) exists. Fixit is a lightweight framework on top of libCST that removes the boilerplate and facilitates testing of linting rules. The code examples in this article use libCST in combination with Fixit.

Creating a libCST lint rule to find and report HTTP requests without timeouts

First, let's have a look at the problematic code:

get("http://www.example.com")

This call of the get function does not use a timeout keyword argument and potentially blocks forever. We can look at the function call's concrete syntax tree using libcst.parse_expression:

Call(
    func=Name(
        value='get',
        lpar=[],
        rpar=[],
    ),
    args=[
        Arg(
            value=SimpleString(
                value='"http://www.example.com"',
                lpar=[],
                rpar=[],
            ),
            keyword=None,
            equal=MaybeSentinel.DEFAULT,
            comma=MaybeSentinel.DEFAULT,
            star='',
            whitespace_after_star=SimpleWhitespace(
                value='',
            ),
            whitespace_after_arg=SimpleWhitespace(
                value='',
            ),
        ),
    ],
    lpar=[],
    rpar=[],
    whitespace_after_func=SimpleWhitespace(
        value='',
    ),
    whitespace_before_args=SimpleWhitespace(
        value='',
    ),
)

The expression consists of a call to a function named get. The function call has only one argument which is the URL to which the request is sent. In order to detect occurences of the get call that violate our constraint, we create a new class that inherits from CstLintRule:

from fixit import CstLintRule
import libcst as cst

class UseRequestTimeouts(CstLintRule):
    MESSAGE = "HTTP calls must specify a timeout"

    def visit_Call(self, node: cst.Call) -> None:
        pass

The class specifies a message that is displayed in case of rule violation. It also implements the method visit_Call which is invoked for every function call in the analyzed source code. For starters, we check if the function call in question is a call to the request library's get function:

import libcst.matchers as m

    def visit_Call(self, node: cst.Call) -> None:
        is_http_call = m.matches(node.func, m.Name("get"))

If you are familiar with the Hamcrest (opens new window) framework, you will know how to work with matchers. LibCST provides so called matchers for this purpose. Matchers provide a declarative way to check for specific attributes of our CST. They form a tree on their own which is "overlayed" onto the subtree of the current node. The matcher tree matches if the node types and their attributes are identical in both trees.

In our case, the matcher tree consists of a single node, the Name matcher. We check if it matches called function. The resulting boolean is True for all calls to functions named get. In the next step we need to determine whether the call has a timout argument or not:

has_timeout_argument = m.matches(node, m.Call(
    func=m.DoNotCare(),
    args=[
        m.Arg(
            keyword=m.Name("timeout"),
            value=m.DoNotCare(),
        )
    ]
))

For this purpose, we create a matcher for the Call node which has a keyword argument named timeout. Note that we match the function of the Call node again using the special DoNotCare matcher. We cannot leave it out, because libCST matchers need to be exhaustive. That also means that the matcher we just constructed only matches function calls that have a single keyword argument called timeout. Instead, we want to match function calls where any of the arguments is a timeout argument. For that purpose, we surround the argument matcher with the ZeroOrMore wildcard matchers:

has_timeout_argument = m.matches(node, m.Call(
    func=m.DoNotCare(),
    args=[
        m.ZeroOrMore(),
        m.Arg(
            keyword=m.Name("timeout")
            value=m.DoNotCare(),
        ),
        m.ZeroOrMore(),
    ]
))

So far so good. We can now determine two different properties for all function calls: (1) Whether the function name is get, and (2) whether the function call has a keyword argument named timeout. Let's bring everything together and show a warning for all get calls that do not have a timeout argument:

from fixit import CstLintRule
import libcst as cst
import libcst.matchers as m


class UseRequestTimeouts(CstLintRule):
    MESSAGE = "HTTP calls must specify a timeout"

    def visit_Call(self, node: cst.Call) -> None:
        is_http_call = m.matches(node.func, m.Name("get"))
        has_timeout_argument = m.matches(node, m.Call(
            func=m.DoNotCare(),
            args=[
                m.ZeroOrMore(),
                m.Arg(
                    keyword=m.Name("timeout"),
                    value=m.DoNotCare(),
                ),
                m.ZeroOrMore(),
            ]
        ))
        if is_http_call and not has_timeout_argument:
            self.report(node)

Issuing a warning is as simple as calling self.report for the node that is being analyzed. But does our rule work as intended?

Testing the libCST rule using Fixit

Fixit provides a comfortable way to test lint rules. In particular, it is capable of generating test cases at runtime compatible with Python's unittest module. Runtime test case generation is as simple as calling add_lint_rule_tests_to_module with the correct arguments in one of the test modules, e.g. tests/__init__.py:

from pathlib import Path

from fixit import add_lint_rule_tests_to_module

from my_project import UseRequestTimeouts


add_lint_rule_tests_to_module(
    globals(),
    {UseRequestTimeouts},
)

After setting up the test case generation, the test examples for the lint rule can be defined as class attributes of the rule. Just like there is a MESSAGE attribute, the VALID and INVALID attributes are lists with code examples:

from fixit import InvalidTestCase, ValidTestCase

class UseRequestTimeouts(CstLintRule):
    MESSAGE = "HTTP calls must specify a timeout"
    VALID = [
        ValidTestCase('get("http://www.example.com", timeout=5.0)')
    ]
    INVALID = [
        InvalidTestCase('get("http://www.example.com")')
    ]

One test case will be generated for each valid and invalid example and they will be run as part of the test suite:

$ python -m unittest tests
..
-------------------------------------
Ran 2 tests in 0.052s

OK

Creating a libCST codemod to add default timeouts

At this point, the functionality of our linting rule is on par with a custom rule for flake8 or pylint. But the best thing is yet to come! We will create a libCST codemod to automatically add a five second timeout to get requests that do not specify a timeout. First, we need to adjust our code examples. We specify the desired state after applying the codemod via the keyword argument expected_replacement for each invalid example:

class UseRequestTimeouts(CstLintRule):
    VALID = [
        ValidTestCase('get("http://www.example.com", timeout=5.0)'),
    ]
    INVALID = [
        InvalidTestCase('get("http://www.example.com")', expected_replacement='get("http://www.example.com", timeout=5.0)'),
    ]

Implementing the codemod requires us to modify the existing CST accordingly. We create a new Arg node that represents the timeout keyword argument. Note that we are now using libCST node classes, not matchers: Specifying equal is optional. Its purpose is to override libCST behavior. By default libCST puts spaces before and after the equals sign.

if is_http_call and not has_timeout_argument:
    timeout_argument = cst.Arg(
        keyword=cst.Name("timeout"),
        value=cst.Float("5.0"),
        equal=cst.AssignEqual(
            whitespace_before=cst.SimpleWhitespace(""),
            whitespace_after=cst.SimpleWhitespace(""),
        ),
    )

This new Arg node can be used to replace the argument list of the existing Call node. The method node.with_changes returns a copy of the old Call node where the specified attributes have been replaced. The replacement node can simply be added to the report call:

call_with_timeout = node.with_changes(
    args=[*node.args, timeout_argument]
)
self.report(node, replacement=call_with_timeout)

It is left as an excercise to the reader to extend the example to work for other HTTP methods, such as post, and for functions that are called as member attributes of the requests module, such as requests.get(…). Pro tip: Matchers can be composed using the & or | operators to create a logical "all of" or "one of" matcher term.

Recap

The article presented an automated way to detect and fix violations of coding convetions using libCST and Fixit. Compared to the traditional approach where coding conventions are enforced manually by upholding them during the programming process or in reviews, this approach has numerous advantages:

• Humans occasionally fail to address a violation. The automated approach is expected to have a lower error rate.
• Automation reduces cognitive load during programming and reviews.
• Coding conventions often live in the heads of the team members. However, when writing a linting rule and an associated codemod, these rules are externalized. Therefore, the conventions outlive the team members, which is especially benefitial in development teams with high turnover.
• At the same time, onboarding for new developers becomes easier, because coding conventions can be learned gradually.
• Linting rules like the one that ensures request timeouts represent architectural fitness functions (opens new window). They ascertain that specific properties of our sofware system hold. A code review can only make the assertion at one point in time. The linting rule can make the assertion every time it is run.

The drawback of the presented approach is that linting rules consist of code and that adds to the maintenance cost of the project.

Despite this, I recommend that you consider adding a new linting rule every time your team agrees on some property of the software system or the source code. The added maintenance cost is probably a small price to pay for faster, less error prone development and improved knowledge transfer.