The current implementation of `build_json` requires that no exceptions be thrown

[dscottboggs]

The current implementation of build_json requires that no exceptions be thrown while the response is being built. Until now, the only resolution to this I could think of was building to an intermediary IO and copying to context.response after it's built. That would be a big performance hit in a crucial spot. However, talking another look at my implementation I was thinking that there is one alternative, although it is less than ideal....suggestions are welcome, but I'd recommend we discuss this in a separate PR, so that I can also begin work on other routes while we work out the details.

def build_json(response : IO)
  error : Exception? = nil
  JSON.build response do |json|
    json.object do
      json.field "data" do
        json.object do
          yield json
        rescue e
          error = e
        end
      end
      if error
        json.field "errors" do
          ErrorResponse.new(error).errors.to_json json
        end
      end
    end
  end
end

I realize this would require specifying a partial succesful response, but there's no way to rewind an HTTP::Server::Response because it's streamed out to the client on the fly.

Originally posted by @dscottboggs in #10

For the package routes, I (think) I successfully avoided cases which raise, so that's an option as well, but it's...fragile, and I don't like it, especially not long-term

[dscottboggs]

Options:

1. Build data internally before sending it out.
   1. Store the JSON text itself in an IO of some sort, then copy that directly to the response.
   2. Store an intermediary data type. Additional data structures would need defined for each route.
2. Allow an exception to be stored, and optionally added to a (partial) successful response.
   1. See the original comment for implementation. This is the only single-channel option that will respond with valid JSON that I can think of.
   2. Respond with the errors in a response header

Advantages:

1. Straightforward to implement
   1. the most simple and quick to implement (of the 1. options)
   2. (probably) more performant than storing the raw JSON text, at least in terms of memory. Benchmarks would be needed to determine the performance impact of this option.
2. negligible performance impact on successful response; minor (probably least) performance impact on an error
   1. Performant, simple to implement, and easy to read and understand.
   2. allows arbitrarily placed exception raising, with no regard as to what is in the response body.

Disadvantages:

1. Performance
   1. the largest performance cost, especially when it comes to memory when forming large responses.
   2. tedious to implement
2. Not compliant with the JSON-API standard, which states "The members data and errors MUST NOT coexist in the same document.".
   1. None other than the general 2. disadvantages
   2. This is a highly unusual course of action and not used by any other application that I know of

Feel free to edit this comment to add options, advantages, and disavantages. However, keep the lists in order so that we can reference them in a discussion below. If an option is to be dismissed, strike it out.

[j8r]

There is a third option, storing to an object instead of an IO, then doing #to_json

I think the first option is the better one.

[dscottboggs]

Isn't that Option 1.ii?

[dscottboggs]

I prefer option 2.i for technical reasons. I find myself questioning the validity of jsonapi.org's standardization process... was there an RFC process, or did someone just come up with it and post it online? Do other APIs actually follow this standard? If jsonapi.org is a standard we are writing to, then obviously the 2. options are out. However, I think the simplicity of implementation leads 1.i to be the better remaining option, at least for now. Perhaps we could set it up with 1.i, but leave this issue open and implement/benchmark a 1.ii implementation. This is all it would take to implement 1.i

def build_json(context : HTTP::Server::Context)
  buffer = IO::Memory.new
  JSON.build buffer do |json|
    json.object do
      json.field("data") { json.object { yield json } }
    end
  end
rescue e
  raise InternalServerError.new context, cause: e
else
  IO.copy buffer, context.response
end

[j8r]

JSON api is right about not having both errors and data , that's also what most other APIs do. Either an error or a success response is returned, not both at the same time.