Possibility to handle response with @response object

[blaind]

First of all, thanks for a great package. Very easy to setup and functions well.

Now to the problem: I'd like to use restivus for both REST-stuff, and also for a few lower-level HTTP calls where I'm writing back binary data in chunks at a callback. I know that's not really anymore the scope of this package (REST), but as it 'almost' seems to work, would it be possible to implement?

Basically, in get() function, I hooked to @response object, and write(..) data there. After it's done, I do @response.end().

  Restivus.addRoute 'test', { authRequired: false }, get: ->
    _this = @
    _this.response.write("test1")
    setTimeout(->
      _this.response.write("test")
      _this.response.end()
    , 1000)
    return null

The above code writes "test1" to the client, but not "test", probably as restivus has already closed the connection. Could there be a parameter that would indicate that method will not return anything and will take care of ending the response?

Also, restivus logs following error:

TypeError: Cannot read property 'body' of null at [object Object].Router.route.action (packages/nimble:restivus/lib/route.coffee:53:24)

[kahmali]

Thank you very much for the kind words!

I think there may be two separate issues here (one on Restivus' end, and one on yours). I definitely intend for users to be able to handle the [low-level responses] with this.response themselves, but you are correct that there is a bug that would prevent this from happening. Let's address that first.

"Could there be a parameter that would indicate that method will not return anything and will take care of ending the response?

Restivus logs following error:

TypeError: Cannot read property 'body' of null at [object Object].Router.route.action (packages/nimble:restivus/lib/route.coffee:53:24)"

I can't think of any clean way to use a parameter in the endpoint to cause Restivus to ignore the data returned from the endpoint. If you have any suggestions (preferably with some example code), I would love to hear them. I think there may be an acceptable way around this though.

As it stands, returning null from an endpoint is treated as an error. My thought there was that something should always be returned from a REST endpoint. Of course, that was an oversight, since a user handling the low-level response should be able to return their own data and prevent Restivus from attempting to send another response, causing an error (as you suggested). Intuitively, you would think to just return null in that situation (as you did), since you've already handled the response, and after giving it some thought I think that's the right way to go.

Handling the null response instead of treating it like an error was quite straight forward, and I've already implemented and tested that on a local branch. This allows you to handle the response manually with this.response. Now, when an endpoint returns null, Restivus ensures the connection is closed and does not attempt to send any response of it's own. I even added a helper function to the endpoint context to make this look a little cleaner: this.end() (all it does is return null, but it looks a bit nicer and better declares your intention). I think it would be suitable for your needs as is, but there are still a few things to consider before publishing an update.

Right now I'm in the process of implementing some pretty handy response hooks (to allow you to call functions at predefined points before, during, and after execution of your endpoints), and one of the hooks is the onBeforeResponse(data) hook, which I would lose control over if users handled the response from within their endpoint (by calling this.response.write()). I've thought about this some and I just don't see a clean way to avoid this. Supporting the automated hook would require me to break up every endpoint action and it's response (so I could call the hook in between), which would introduce unnecessary complexity to the Restivus API. The only reasonable solution I've come up with is simply canceling my own attempt to call the hook whenever the response has already been initiated within the endpoint (I can check that with this.response.headersSent, which returns true once this.response.write() has been called). That means the user would be required to call the onBeforeResponse hook manually before beginning their response if they want the hook to be called from that endpoint with its intended behavior. If I don't cancel the hook on my end, it would have inconsistent behavior (sometimes being called after the response), and I couldn't sleep well at night if that were the case. If losing the automated endpoint hook seems like an acceptable tradeoff to you, let me know. Otherwise, if you have any suggestions on how to better handle this please let me know.

I've left it so that returning undefined from an endpoint is still an error. Is that confusing? I'm a little weary of just closing the connection when null or undefined are returned. I think that would mask errors in the REST API. I think I would prefer to continue to treat undefined as an error. What I can do is to return a more specific error there, instead of just letting the error fall through.

That was a lot to take in, but that covers the issues being caused by Restivus. As for the issue on your end:

  Restivus.addRoute 'test', { authRequired: false }, get: ->
    _this = @
    _this.response.write("test1")
    setTimeout(->
      _this.response.write("test")
      _this.response.end()
    , 1000)
    return null

"The above code writes "test1" to the client, but not "test", probably as restivus has already closed the connection."

The reason that "test" is not returned is because setTimeout() is asynchronous and your endpoint function will always return before the asynchronous callback function runs. Meteor has first- and third-party options for solving this issue. Please check out my response to Issue #11 for a detailed explanation of how to resolve this.

I will push these changes up soon for you to test out. I'll keep you posted. Let me know if any of that was unclear, or if you have any other questions or suggestions.

[kahmali]

Sorry for not publishing this update yet. I wasn't really happy with simply returning null from an endpoint to indicate that the response had been handled manually. Returning null may be done by accident, and in that case the response would be sent back with an empty body and no indication of any error. Instead, I would prefer to continue to treat a null return value as an error. I spoke with @anthonyreid99 and we came up with a much cleaner solution. I won't bore you with the details just yet, but I should be able to implement it within the next few days and get this update out. The API should be very close to the one proposed here. I'll keep you posted. Sorry about the delay on this.

[kahmali]

Sorry about all of those commit references. Just had to rebase a bunch to get the markdown right for the docs. Forgot that I had referenced the issue.

Anywho, the latest release, v0.6.3 (along with my suggestions above) should fix this issue. I added a section to the docs to explain how to handle the response manually from within your endpoints. The only difference is that you just need to call this.done() after you're done handling the response, and Restivus won't get in your way anymore.

Please let me know if you have any other questions or issues. Thank you for taking the time to report this!

[blaind]

Thank you!