ash-framework / roadmap

0 stars 0 forks source link

http status codes #30

Closed digitalsadhu closed 7 years ago

digitalsadhu commented 8 years ago
digitalsadhu commented 7 years ago

@pnw thoughts on how we should automatically return error codes from routes? Also how we should expose the ability to return a specific http status code to the user?

digitalsadhu commented 7 years ago

Thinking something like:

class MyRoute extends Ash.Route {
  model () {
    this.status = 404
  }
}

If status is not explicitly set then we would do our best to work out status from the route details.

Thoughts @pnw?

pnw commented 7 years ago

How about this:

class MyRoute extends Ash.Route {
  model () {
    this.response.status = 404
    this.response.headers['Content-Type'] = this.headers['Content-Type']
    return {error: 'Custom Error Message'}
  }
}

Explanation: I've added that second line to illustrate why I think we need to set response attributes on a response object instead of just on this. I know you've expressed desire to keep the request/response hidden from the user if possible, but the more I think about it, I think we might need to just accept that's going to make things really complicated for us, especially when we get into things like streaming requests and responses. It would also require us to re-implement just about every feature of express requests/responses for the sake of not interacting directly with the request/response.

Judging by what I've seen so far, this.attr tends to be the way to access request attributes like query parameters and the request body. In the case of headers (and any other attribute that's shared between request and response), we need to differentiate between request and response - hence the this.response.headers =. If we need to do something like that for headers, then it follows that we should do the same for other response attributes, like statusCode, for the sake of consistency.

digitalsadhu commented 7 years ago

Ok, I've tried for several days to figure out how to not have a response object and handle something like headers but everything i think of is worse so i'm going to make the move over to route has request props set on it and a response object

pnw commented 7 years ago

IMO just having request and response objects easily available on the Routes is a cheap win that will take care of a lot of edge cases. If it makes sense to obscure those things, I think it makes sense to do that later as the framework matures and functionality is built out. One step at a time kind of argument.