-
Notifications
You must be signed in to change notification settings - Fork 524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
StatusCode support for ending RoutingContext #2146
Comments
Why not? /**
* Shortcut to the response end with a given status code.
* @return future
*/
default Future<Void> end(int statusCode) {
return response().setStatusCode(statusCode).end();
}
/**
* Shortcut to the response end with a given status code.
* See {@link #end(int)}
*/
@Fluent
default RoutingContext end(int statusCode, Handler<AsyncResult<Void>> handler) {
end(statusCode).onComplete(handler);
return this;
} Which would mean: ctx.end(204); |
We would not want a coupling to netty to support e.g. |
@thced although I proposed it, now that I'm looking at the idea after giving it a bit of thought, I feel this isn't really right. I see 2 ways of achieving the same result, and I'd prefer not to. I think a better solution would be to stick to the builder API style for consistency: ctx.statusCode(204).end();
// or
ctx.statusCode(HttpResponseStatus.ACCEPTED).end(); Which means we could create a few shortcuts in the statusCode(int|Enum); // delegates to response.setStatusCode(int) Note that we should not use netty specific enums, as the goal of vert.x core is to abstract the underlying transport and protect agains API changes. So, it needs a new enum wrapper, as we already have for other things like http headers. |
Also like @vietj mentioned on res.
.write("hello")
.write("vert.x")
.end() And this point, a user calling the end with a status code argument, will fail because the response has already been written. |
Good input, thanks! I can perhaps adjust this PR a bit to solve it using that proposed direction instead? |
in vertx core HttpClientRequest we added a send() method that takes care of sending something like a buffer or a stream. default Future<HttpClientResponse> send() {
end();
return response();
}
default Future<HttpClientResponse> send(Buffer body) {
end(body);
return response();
}
default Future<HttpClientResponse> send(ReadStream<Buffer> body) {
MultiMap headers = headers();
if (headers == null || !headers.contains(HttpHeaders.CONTENT_LENGTH)) {
setChunked(true);
}
body.pipeTo(this);
return response();
} |
it is also on HttpServerResponse |
So, basically the issue is not whether we can check if header has been written or not, the issue you see is the inconsistency in the api this might bring? |
To end a RoutingContext response, one has to specify:
An alternative, more pleasant-to-the-eye solution, could be to let RoutingContext accept ending with a Integer or a (netty) HttpResponseStatus, similar to this:
Reasonable?
The text was updated successfully, but these errors were encountered: