Skip to content

Commit

Permalink
4.x: UncheckedIOException no longer a special case (helidon-io#9206)
Browse files Browse the repository at this point in the history 
* No longer using UncheckedIOException as a special case for the server.
Server I/O failures now throw a ServerConnectionException, and that extends the CloseConnectionException (which already has its special handling).

This change allows user to handle UncheckedIOException as if it is any other throwable.

* Update webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1Connection.java


Signed-off-by: Tomas Langer <[email protected]>
Co-authored-by: Daniel Kec <[email protected]>
  • Loading branch information
@tomas-langer @danielkec
tomas-langer and danielkec authored Aug 29, 2024
1 parent 368f300 commit 1f30d97
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 87 deletions.
12 changes: 6 additions & 6 deletions ...profile/server/src/test/java/io/helidon/microprofile/server/JaxRsApplicationPathTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
* Copyright (c) 2023, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,9 +16,6 @@

package io.helidon.microprofile.server;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

import java.util.HashSet;
import java.util.Set;

Expand All @@ -39,9 +36,11 @@
import jakarta.ws.rs.core.UriInfo;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

class JaxRsApplicationPathTest {

private static Server server;
Expand Down Expand Up @@ -87,7 +86,8 @@ void emptyParamTest() {
public void testEncoded() {
String getResponse = client.target("http://localhost:" + port)
.path("/ApplicationPath!/Resource/encoded").queryParam("query", "%dummy23+a")
.request().get(String.class);
.request()
.get(String.class);
assertThat(getResponse, is("true:%25dummy23%2Ba"));
}

Expand Down
79 changes: 42 additions & 37 deletions webserver/http2/src/main/java/io/helidon/webserver/http2/Http2ServerResponse.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -32,6 +32,7 @@
import io.helidon.http.Status;
import io.helidon.http.http2.Http2Headers;
import io.helidon.webserver.ConnectionContext;
import io.helidon.webserver.ServerConnectionException;
import io.helidon.webserver.http.ServerResponseBase;

class Http2ServerResponse extends ServerResponseBase<Http2ServerResponse> {
Expand Down Expand Up @@ -74,51 +75,55 @@ public Http2ServerResponse header(Header header) {

@Override
public void send(byte[] entityBytes) {
if (outputStreamFilter != null) {
// in this case we must honor user's request to filter the stream
try (OutputStream os = outputStream()) {
os.write(entityBytes);
} catch (IOException e) {
throw new UncheckedIOException(e);
try {
if (outputStreamFilter != null) {
// in this case we must honor user's request to filter the stream
try (OutputStream os = outputStream()) {
os.write(entityBytes);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
return;
}
return;
}

if (isSent) {
throw new IllegalStateException("Response already sent");
}
if (streamingEntity) {
throw new IllegalStateException("When output stream is used, response is completed by closing the output stream"
+ ", do not call send().");
}
isSent = true;
if (isSent) {
throw new IllegalStateException("Response already sent");
}
if (streamingEntity) {
throw new IllegalStateException("When output stream is used, response is completed by closing the output stream"
+ ", do not call send().");
}
isSent = true;

// handle content encoding
byte[] bytes = entityBytes(entityBytes);
// handle content encoding
byte[] bytes = entityBytes(entityBytes);

headers.setIfAbsent(HeaderValues.create(HeaderNames.CONTENT_LENGTH,
true,
false,
String.valueOf(bytes.length)));
headers.setIfAbsent(HeaderValues.create(HeaderNames.DATE, true, false, DateTime.rfc1123String()));
headers.setIfAbsent(HeaderValues.create(HeaderNames.CONTENT_LENGTH,
true,
false,
String.valueOf(bytes.length)));
headers.setIfAbsent(HeaderValues.create(HeaderNames.DATE, true, false, DateTime.rfc1123String()));

Http2Headers http2Headers = Http2Headers.create(headers);
http2Headers.status(status());
headers.remove(Http2Headers.STATUS_NAME, it -> ctx.log(LOGGER,
System.Logger.Level.WARNING,
"Status must be configured on response, "
+ "do not set HTTP/2 pseudo headers"));

Http2Headers http2Headers = Http2Headers.create(headers);
http2Headers.status(status());
headers.remove(Http2Headers.STATUS_NAME, it -> ctx.log(LOGGER,
System.Logger.Level.WARNING,
"Status must be configured on response, "
+ "do not set HTTP/2 pseudo headers"));
boolean sendTrailers = request.headers().contains(HeaderValues.TE_TRAILERS) || headers.contains(HeaderNames.TRAILER);

boolean sendTrailers = request.headers().contains(HeaderValues.TE_TRAILERS) || headers.contains(HeaderNames.TRAILER);
http2Headers.validateResponse();
bytesWritten += stream.writeHeadersWithData(http2Headers, bytes.length, BufferData.create(bytes), !sendTrailers);

http2Headers.validateResponse();
bytesWritten += stream.writeHeadersWithData(http2Headers, bytes.length, BufferData.create(bytes), !sendTrailers);
if (sendTrailers) {
bytesWritten += stream.writeTrailers(Http2Headers.create(trailers));
}

if (sendTrailers) {
bytesWritten += stream.writeTrailers(Http2Headers.create(trailers));
afterSend();
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed writing entity", e);
}

afterSend();
}

@Override
Expand Down
75 changes: 50 additions & 25 deletions webserver/http2/src/main/java/io/helidon/webserver/http2/Http2ServerStream.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -57,6 +57,7 @@
import io.helidon.webserver.CloseConnectionException;
import io.helidon.webserver.ConnectionContext;
import io.helidon.webserver.Router;
import io.helidon.webserver.ServerConnectionException;
import io.helidon.webserver.http.HttpRouting;
import io.helidon.webserver.http2.spi.Http2SubProtocolSelector;
import io.helidon.webserver.http2.spi.SubProtocolResult;
Expand Down Expand Up @@ -195,23 +196,27 @@ public boolean rstStream(Http2RstStream rstStream) {

@Override
public void windowUpdate(Http2WindowUpdate windowUpdate) {
//5.1/3
if (state == Http2StreamState.IDLE) {
String msg = "Received WINDOW_UPDATE for stream " + streamId + " in state IDLE";
Http2GoAway frame = new Http2GoAway(0, Http2ErrorCode.PROTOCOL, msg);
writer.write(frame.toFrameData(clientSettings, 0, Http2Flag.NoFlags.create()));
throw new Http2Exception(Http2ErrorCode.PROTOCOL, msg);
}
//6.9/2
if (windowUpdate.windowSizeIncrement() == 0) {
Http2RstStream frame = new Http2RstStream(Http2ErrorCode.PROTOCOL);
writer.write(frame.toFrameData(clientSettings, streamId, Http2Flag.NoFlags.create()));
}
//6.9.1/3
long size = flowControl.outbound().incrementStreamWindowSize(windowUpdate.windowSizeIncrement());
if (size > WindowSize.MAX_WIN_SIZE || size < 0L) {
Http2RstStream frame = new Http2RstStream(Http2ErrorCode.FLOW_CONTROL);
writer.write(frame.toFrameData(clientSettings, streamId, Http2Flag.NoFlags.create()));
try {
//5.1/3
if (state == Http2StreamState.IDLE) {
String msg = "Received WINDOW_UPDATE for stream " + streamId + " in state IDLE";
Http2GoAway frame = new Http2GoAway(0, Http2ErrorCode.PROTOCOL, msg);
writer.write(frame.toFrameData(clientSettings, 0, Http2Flag.NoFlags.create()));
throw new Http2Exception(Http2ErrorCode.PROTOCOL, msg);
}
//6.9/2
if (windowUpdate.windowSizeIncrement() == 0) {
Http2RstStream frame = new Http2RstStream(Http2ErrorCode.PROTOCOL);
writer.write(frame.toFrameData(clientSettings, streamId, Http2Flag.NoFlags.create()));
}
//6.9.1/3
long size = flowControl.outbound().incrementStreamWindowSize(windowUpdate.windowSizeIncrement());
if (size > WindowSize.MAX_WIN_SIZE || size < 0L) {
Http2RstStream frame = new Http2RstStream(Http2ErrorCode.FLOW_CONTROL);
writer.write(frame.toFrameData(clientSettings, streamId, Http2Flag.NoFlags.create()));
}
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed to write window update", e);
}
}

Expand Down Expand Up @@ -336,7 +341,11 @@ int writeHeaders(Http2Headers http2Headers, boolean endOfStream) {
flags = Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS);
}

return writer.writeHeaders(http2Headers, streamId, flags, flowControl.outbound());
try {
return writer.writeHeaders(http2Headers, streamId, flags, flowControl.outbound());
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed to write headers", e);
}
}

int writeHeadersWithData(Http2Headers http2Headers, int contentLength, BufferData bufferData, boolean endOfStream) {
Expand All @@ -353,10 +362,14 @@ int writeHeadersWithData(Http2Headers http2Headers, int contentLength, BufferDat
Http2Flag.DataFlags.create(endOfStream ? Http2Flag.END_OF_STREAM : 0),
streamId),
bufferData);
return writer.writeHeaders(http2Headers, streamId,
Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS),
frameData,
flowControl.outbound());
try {
return writer.writeHeaders(http2Headers, streamId,
Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS),
frameData,
flowControl.outbound());
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed to write headers", e);
}
}

int writeData(BufferData bufferData, boolean endOfStream) {
Expand All @@ -373,18 +386,26 @@ int writeData(BufferData bufferData, boolean endOfStream) {
streamId),
bufferData);

writer.writeData(frameData, flowControl.outbound());
try {
writer.writeData(frameData, flowControl.outbound());
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed to write frame data", e);
}
return frameData.header().length() + Http2FrameHeader.LENGTH;
}

int writeTrailers(Http2Headers http2trailers) {
writeState = writeState.checkAndMove(WriteState.TRAILERS_SENT);
streams.remove(this.streamId);

try {
return writer.writeHeaders(http2trailers,
streamId,
Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM),
flowControl.outbound());
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed to write trailers", e);
}
}

void write100Continue() {
Expand All @@ -393,10 +414,14 @@ void write100Continue() {

Header status = HeaderValues.createCached(Http2Headers.STATUS_NAME, 100);
Http2Headers http2Headers = Http2Headers.create(WritableHeaders.create().add(status));
writer.writeHeaders(http2Headers,
try {
writer.writeHeaders(http2Headers,
streamId,
Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS),
flowControl.outbound());
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed to write 100-Continue", e);
}
}
}

Expand Down
49 changes: 49 additions & 0 deletions ...er/tests/webserver/src/test/java/io/helidon/webserver/tests/UncheckedIoExceptionTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.helidon.webserver.tests;

import java.io.IOException;
import java.io.UncheckedIOException;

import io.helidon.http.Status;
import io.helidon.webclient.http1.Http1Client;
import io.helidon.webserver.http.HttpRules;
import io.helidon.webserver.testing.junit5.ServerTest;
import io.helidon.webserver.testing.junit5.SetUpRoute;

import org.junit.jupiter.api.Test;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;

@ServerTest
class UncheckedIoExceptionTest {
@SetUpRoute
static void routing(HttpRules rules) {
rules.get("/fail", ((req, res) -> {
throw new UncheckedIOException("My Exception", new IOException("Outbound client failure"));
}));
}

@Test
void testUncheckedIoTreatedAsAnyOther(Http1Client client) {
var response = client.get("/fail")
.request(String.class);

assertThat(response.status(), is(Status.INTERNAL_SERVER_ERROR_500));
}
}
28 changes: 23 additions & 5 deletions webserver/webserver/src/main/java/io/helidon/webserver/ConnectionHandler.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,6 +25,7 @@
import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Semaphore;
import java.util.function.Supplier;

import javax.net.ssl.SSLSocket;

Expand Down Expand Up @@ -130,7 +131,7 @@ public final void run() {
helidonSocket = PlainSocket.server(socket, channelId, serverChannelId);
}

reader = new DataReader(helidonSocket);
reader = new DataReader(new MapExceptionDataSupplier(helidonSocket));
writer = SocketWriter.create(listenerContext.executor(), helidonSocket,
listenerContext.config().writeQueueLength());
} catch (Exception e) {
Expand Down Expand Up @@ -167,12 +168,12 @@ public final void run() {
helidonSocket.log(LOGGER, WARNING, "escaped Request exception", e);
} catch (HttpException e) {
helidonSocket.log(LOGGER, WARNING, "escaped HTTP exception", e);
} catch (ServerConnectionException e) {
// socket exception - the socket failed, probably killed by OS, proxy or client
helidonSocket.log(LOGGER, TRACE, "server I/O issue", e);
} catch (CloseConnectionException e) {
// end of request stream - safe to close the connection, as it was requested by our client
helidonSocket.log(LOGGER, TRACE, "connection close requested", e);
} catch (UncheckedIOException e) {
// socket exception - the socket failed, probably killed by OS, proxy or client
helidonSocket.log(LOGGER, TRACE, "received I/O exception", e);
} catch (Exception e) {
helidonSocket.log(LOGGER, WARNING, "unexpected exception", e);
} finally {
Expand Down Expand Up @@ -323,4 +324,21 @@ private void closeChannel() {
helidonSocket.log(LOGGER, TRACE, "Failed to close socket on connection close", e);
}
}

private static class MapExceptionDataSupplier implements Supplier<byte[]> {
private final HelidonSocket helidonSocket;

private MapExceptionDataSupplier(HelidonSocket helidonSocket) {
this.helidonSocket = helidonSocket;
}

@Override
public byte[] get() {
try {
return helidonSocket.get();
} catch (UncheckedIOException e) {
throw new ServerConnectionException("Failed to get data from socket", e);
}
}
}
}
Loading

0 comments on commit 1f30d97

Please sign in to comment.