HTTP redirect throws Error, should throw Exception

[cbatson]

On this line of http_impl.dart, a StateError is thrown if the response is a redirect and no Location header was present in the response:

      String? location = headers.value(HttpHeaders.locationHeader);
      if (location == null) {
        throw StateError("Response has no Location header for redirect");
      }
      url = Uri.parse(location);

According to the documentation for the Error class (of which StateError is a subclass):

Error objects thrown in the case of a program failure.

An Error object represents a program failure that the programmer should have avoided.

Examples include calling a function with invalid arguments, or even with the wrong number of arguments, or calling it at a time when it is not allowed.

These are not errors that a caller should expect or catch — if they occur, the program is erroneous, and terminating the program may be the safest response.

However, the presence -- or lack thereof -- of a specific header in a response from a remote server is not something that I, as the Dart programmer, could have avoided. Nor does it mean the "program is erroneous."

It is thus my opinion that this situation should throw an object which is a descendent of Exception (perhaps, for example, a RedirectException such as appears just a few lines further in http_impl.dart), and not a descendent of Error.

It makes little sense to use on StateError to catch this run-time issue.

See also:

• Error vs. Exception in Dart
• https://groups.google.com/a/dartlang.org/g/misc/c/lx9CXiV3o30/m/s5l_PwpHUGAJ
• Document Error class in dart:core #16192

Community note: Please tap the "thumbs-up" on this issue if it is affecting you and you'd like it to be prioritized.

[mkustermann]

/cc @brianquinlan @lrhn

[lrhn]

I agree with the conclusion.

It technically is a state issue, the local HttpClientResponse has an invalid state (a code which suggests redirect, but no redirect target), but it's not something the user can easily check for, and something they won't expect to happen. If the isRedirect returns true, the redirect method should work, and if it doesn't, it's not a programming error in this program, but an invalid response from the server.

The alternative would be to make isRedirect check that the required header is there, and return false if not.
Then you're in an equally confusing state where the response code might be HttpStatus.seeOther, but not isRedirect, so what is it then?

Or document very cleraly that any caller of redirect without a Uri argument must first check that the response has an HttpHeaders.locationHeader. Which nobody will do, because any well-behaved server will have one, and the Dart HttpServer sets it for you when redirecting, so your tests won't ever trigger that case.

So, throwing an exception seems reasonable. Not optimal, but this is a situation that shouldn't happen, and which the program author is not the cause of.

Next question is which exception. The RedirectException seems like a slam-dunk, with a message of "No target URI provided".

And then still document this behavior in the method docs:

/// If [uri] is omitted, the redirect target is taken from the [HttpHeaders.locationHeader] value
/// in this response. If the response has no such header, a [RedirectException] is thrown.
/// If the header value is not a valid URI, as determined by [Uri.parse], a [FormatException] is thrown.

In general, always document all exceptions that can possibly be thrown in your public API.

[a-siva]

Any updates on this ?

[brianquinlan]

Fixed in Ib7ea9503dd58c0d1c233a85d92d4feb0648a9022