Redirects are considered errors

[wasamasa]

The response handling is pretty rudimentary. Everything that's not status 200 is considered an error, which is just wrong:

• A 1xx response might be shown, these shouldn't occur though in practice (100 is for an incomplete request, 101 for switching to websocket communication)
• There's other 2xx responses, such as 201 after a successful POST request or 204 for an empty response. They should be handled the same as a 200.
• The 3xx responses should be handled differently. The easiest solution is to handle them the same as a 2xx and let the user redo the request with the URL in the Location header. If the user wishes for automatic redirection, the -L argument could be passed to curl. The --max-redirs option helps with limiting the amount of redirects.

In other words, if the status code is smaller than 400, it should be considered successful (ignoring the 1xx status codes for now), otherwise an error. See https://www.rfc-editor.org/rfc/rfc2616#page-39 for further explanations.

[alphapapa]

The response handling is pretty rudimentary.

Yes, it works for basic GET/POST/PUT/DELETE requests, but it isn't comprehensive.

Everything that's not status 200 is considered an error, which is just wrong:

• A 1xx response might be shown, these shouldn't occur though in practice (100 is for an incomplete request, 101 for switching to websocket communication)
• There's other 2xx responses, such as 201 after a successful POST request or 204 for an empty response. They should be handled the same as a 200.
• The 3xx responses should be handled differently. The easiest solution is to handle them the same as a 2xx and let the user redo the request with the URL in the Location header. If the user wishes for automatic redirection, the -L argument could be passed to curl. The --max-redirs option helps with limiting the amount of redirects.

In other words, if the status code is smaller than 400, it should be considered successful (ignoring the 1xx status codes for now), otherwise an error. See https://www.rfc-editor.org/rfc/rfc2616#page-39 for further explanations.

Yes, redirects should especially be handled; I simply haven't had to deal with them yet in my use of this library.

100 and 101, while not errors by the spec, would seem to be effectively errors in this context, no?

The other 20x codes are interesting. Are they used in practice? "206 Partial Content", of course, is used and very important, but I haven't seen the others before, even when, e.g. "The request has been fulfilled and resulted in a new resource being created." And I'm not sure if they can all be handled the same way, e.g. 201 says, "The newly created resource can be referenced by the URI(s) returned in the entity of the response, with the most specific URI for the resource given by a Location header field," which would seem to suggest that it would need to be handled specially. I'm not opposed to doing so, of course--I'd be glad for this library to gain more mature and comprehensive support for HTTP.

[wasamasa]

100 and 101, while not errors by the spec, would seem to be effectively errors in this context, no?

They are unexpected behavior perhaps, but not an error in the HTTP sense. I'd use something like an assertion to notify the user handling them hasn't been implemented yet.

The other 20x codes are interesting. Are they used in practice?

Yes, APIs use them. See the flowchart at the end of http://clojure-liberator.github.io/liberator/doc/decisions.html for an example how.

And I'm not sure if they can all be handled the same way, e.g. 201 says, "The newly created resource can be referenced by the URI(s) returned in the entity of the response, with the most specific URI for the resource given by a Location header field," which would seem to suggest that it would need to be handled specially.

Well, I disagree here. The library already provides a way of choosing whether you just want the body as is, parsed as JSON or the whole request. In the case there is interest in that header, the user should request it. No magic handling required at all.

---

I wrote a very simple hack that implements the suggested strategy of checking whether the HTTP status code is smaller than 400 and ran into a new problem: If curl is passed -L to automatically follow redirects, it prints out the headers for both responses and plz.el only looks at the first header block. Therefore the user would need to handle redirects manually.

[alphapapa]

Redirects should be handled since https://github.com/alphapapa/plz.el/commit/a0a6d623352aa1caee722c16649190611a253cbc and v0.6.

[wasamasa]

Thank you. Is there a reason redirects are only handled when specifying :as 'response? It is kind of different from curl behavior (where by default no redirect handling is performed and one can choose between automatically handling an infinite or finite amount of redirects).

[alphapapa]

Is there a reason redirects are only handled when specifying :as 'response?

That's not the case. The fix in https://github.com/alphapapa/plz.el/commit/a0a6d623352aa1caee722c16649190611a253cbc was for a bug in which redirects were not handled only when using :as 'response. Please see the commit message.

[wasamasa]

Ok, I got confused by the only mention of redirect handling in the README saying "Handle HTTP 3xx redirects when using :as 'response." I kinda regret not adding a test case to the issue, will have to come up with one to check how things behave.

[alphapapa]

I could have worded that more clearly, yes. :)

I added several test cases with that fix. They do all use :as 'response, because that's what the fix was specifically for, but it would be fine to add some more.

[wasamasa]

Alrighty, I've come up with this test case:

(plz 'get "https://httpbin.org/redirect-to?url=/json" :as #'json-read)
;; => ((slideshow (author . "Yours Truly") (date . "date of publication") (slides . [((title . "Wake up to WonderWidgets!") (type . "all")) ((items . ["Why <em>WonderWidgets</em> are great" "Who <em>buys</em> WonderWidgets"]) (title . "Overview") (type . "all"))]) (title . "Sample Slide Show")))

So it does indeed now work as if -L was passed. Good enough for me.