spraypaint.js icon indicating copy to clipboard operation
spraypaint.js copied to clipboard

Don't skip afterFetch in cases where there's no response body.

Open zpencerq opened this issue 5 years ago • 0 comments

The original error was that the afterFetch middleware functions weren't being called because of various short-circuiting logic within Request#_handleResponse.

Behavior changes

  • When sending a DELETE and getting a 200 response, it will now try to parse that body. Prior to this change-set, it would skip.
  • An invalid JSON document is now determined by both data and meta being undefined rather than just data (aside from a JSON parse error).
    • This is informed by the previous link as well, since only meta being defined is valid.
  • afterFetch calls happen with json=null in the case of a 204 No Content.
    • Presumably this will affect existing middlewares.

Questions:

  • The spec for deleting resources does not specify if a response body MUST be returned. However, you could interpret that "a deletion request has been accepted for processing ... and no content is returned" requires that a 204 be returned by the server. Should we remove the ability for non-204 responses to be empty?

  • The default json body of null was chosen instead of an empty object because it's representing the absence of a body; I figured just {} would be confusing (but I also don't like giving people NPE). Do you think we should introduce some Null Object default that'll explain where it came from (empty body) on field access?

zpencerq avatar Jul 11 '19 02:07 zpencerq