CodeSOD: I Am Not 200

In theory, HTTP status codes should be easy to work with. In the 100s? You’re doing some weird stuff and breaking up large requests into multiple sub-requests. 200s? It’s all good. 300s? Look over there. 400s? What the hell are you trying to do? 500s? What the hell is the server trying to do?

This doesn’t mean people don’t endlessly find ways to make it hard. LinkedIn, for example, apparently likes to send 999s if you try and view a page without being logged in. Shopify has invented a few. Apache has added a 218 “This is Fine“. And then there’s WebDAV, which not only adds new status codes, but adds a whole bunch of new verbs to HTTP requests.

Francesco D sends us a “clever” attempt at handling status codes.

    try {
      HttpRequest.Builder localVarRequestBuilder = {{operationId}}RequestBuilder({{#allParams}}{{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#hasParams}}, {{/hasParams}}headers);
      return memberVarHttpClient.sendAsync(
          localVarRequestBuilder.build(),
          HttpResponse.BodyHandlers.ofString()).thenComposeAsync(localVarResponse -> {
            if (localVarResponse.statusCode()/ 100 != 2) {
              return CompletableFuture.failedFuture(getApiException("{{operationId}}", localVarResponse));
            }
            {{#returnType}}
            try {
              String responseBody = localVarResponse.body();
              return CompletableFuture.completedFuture(
                  responseBody == null || responseBody.isBlank() ? null : memberVarObjectMapper.readValue(responseBody, new TypeReference<{{{returnType}}}>() {})
              );
            } catch (IOException e) {
              return CompletableFuture.failedFuture(new ApiException(e));
            }
            {{/returnType}}
            {{^returnType}}
            return CompletableFuture.completedFuture(null);
            {{/returnType}}
      });
    }

Okay, before we get to the status code nonsense, I first have to whine about this templating language. I’m generally of the mind that generated code is a sign of bad abstractions, especially if we’re talking about using a text templating engine, like this. I’m fine with hygienic macros, and even C++’s templating system for code generation, because they exist within the language. But fine, that’s just my “ok boomer” opinion, so let’s get into the real meat of it, which is this line:

localVarResponse.statusCode()/ 100 != 2

“Hey,” some developer said, “since success is in the 200 range, I’ll just divide by 100, and check if it’s a 2, helpfully truncating the details.” Which is fine and good, except neither 100s nor 300s represent a true error, especially because if the local client is doing caching, a 304 tells us that we can used the cached version.

For Francesco, treating 300s as an error created a slew of failed requests which shouldn’t have failed. It wasn’t too difficult to detect- they were at least logging the entire response- but it was frustrating, if only because it seems like someone was more interested in being clever with math than actually writing good software.

[Advertisement]
Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Source: Read More