Skip to content

Commit b147a62

Browse files
committed
(NETTY-12) Only set the responder state to be sent when it’s going to write to the output channel.
- Also call BodyProducer.onError if exception raised when calling BodyProducer.getContentLength
1 parent 6870ca4 commit b147a62

File tree

2 files changed

+9
-7
lines changed

2 files changed

+9
-7
lines changed

src/main/java/co/cask/http/BasicHttpResponder.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ public BasicHttpResponder(Channel channel, boolean keepAlive) {
6767

6868
@Override
6969
public ChunkResponder sendChunkStart(HttpResponseStatus status, @Nullable Multimap<String, String> headers) {
70-
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
7170
Preconditions.checkArgument((status.getCode() >= 200 && status.getCode() < 210) , "Http Chunk Failure");
7271
HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, status);
7372

@@ -79,14 +78,14 @@ public ChunkResponder sendChunkStart(HttpResponseStatus status, @Nullable Multim
7978
}
8079

8180
boolean responseKeepAlive = setResponseKeepAlive(response);
81+
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
8282
channel.write(response);
8383
return new ChannelChunkResponder(channel, responseKeepAlive);
8484
}
8585

8686
@Override
8787
public void sendContent(HttpResponseStatus status, @Nullable ChannelBuffer content, String contentType,
8888
@Nullable Multimap<String, String> headers) {
89-
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
9089
HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, status);
9190

9291
setCustomHeaders(response, headers);
@@ -100,6 +99,7 @@ public void sendContent(HttpResponseStatus status, @Nullable ChannelBuffer conte
10099
}
101100

102101
boolean responseKeepAlive = setResponseKeepAlive(response);
102+
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
103103
ChannelFuture future = channel.write(response);
104104
if (!responseKeepAlive) {
105105
future.addListener(ChannelFutureListener.CLOSE);
@@ -108,14 +108,15 @@ public void sendContent(HttpResponseStatus status, @Nullable ChannelBuffer conte
108108

109109
@Override
110110
public void sendFile(File file, @Nullable Multimap<String, String> headers) {
111-
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
112111
HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
113112

114113
setCustomHeaders(response, headers);
115114
response.setHeader(HttpHeaders.Names.CONTENT_LENGTH, file.length());
116115

117116
final boolean responseKeepAlive = setResponseKeepAlive(response);
118117

118+
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
119+
119120
// Write the initial line and the header.
120121
channel.write(response);
121122

@@ -144,11 +145,11 @@ public void operationComplete(ChannelFuture future) throws Exception {
144145
@Override
145146
public void sendContent(HttpResponseStatus status, final BodyProducer bodyProducer,
146147
@Nullable Multimap<String, String> headers) {
147-
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
148148
final long contentLength;
149149
try {
150150
contentLength = bodyProducer.getContentLength();
151151
} catch (Throwable t) {
152+
bodyProducer.handleError(t);
152153
// Response with error and close the connection
153154
sendContent(HttpResponseStatus.INTERNAL_SERVER_ERROR,
154155
ChannelBuffers.wrappedBuffer(
@@ -173,6 +174,9 @@ public void sendContent(HttpResponseStatus status, final BodyProducer bodyProduc
173174
boolean responseKeepAlive = setResponseKeepAlive(response);
174175
final ChannelFutureListener completionListener = createBodyProducerCompletionListener(bodyProducer,
175176
responseKeepAlive);
177+
178+
Preconditions.checkArgument(responded.compareAndSet(false, true), "Response has been already sent");
179+
176180
// Streams the data produced by the given BodyProducer
177181
channel.write(response).addListener(new ChannelFutureListener() {
178182

src/main/java/co/cask/http/RequestRouter.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,7 @@ private boolean handleRequest(HttpRequest httpRequest, Channel channel, ChannelH
134134
public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent e) {
135135
String exceptionMessage = "Exception caught in channel processing.";
136136
Throwable cause = e.getCause();
137-
if (!exceptionRaised.get()) {
138-
exceptionRaised.set(true);
139-
137+
if (exceptionRaised.compareAndSet(false, true)) {
140138
if (methodInfo != null) {
141139
LOG.error(exceptionMessage, cause);
142140
methodInfo.sendError(HttpResponseStatus.INTERNAL_SERVER_ERROR, cause);

0 commit comments

Comments
 (0)