Skip to content
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

Remove checked exception from Response.close() and BinaryData.close() #44485

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public Response<?> send(HttpRequest request) {

return new MockHttpResponse(request, success ? 200 : 400) {
@Override
public void close() throws IOException {
public void close() {
closeCalledOnResponse = true;

super.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
import io.clientcore.core.implementation.http.HttpResponse;
import io.clientcore.core.implementation.http.HttpResponseAccessHelper;
import io.clientcore.core.models.binarydata.BinaryData;
import java.io.IOException;
import java.io.UncheckedIOException;

import static io.clientcore.annotation.processor.templating.JavaParserTemplateProcessor.isPrimitiveOrWrapper;

Expand Down Expand Up @@ -121,11 +119,7 @@ public static void generateResponseHandling(BlockStmt body, String returnTypeNam
}

private static void closeResponse(BlockStmt body) {
body.tryAddImportToParentCompilationUnit(IOException.class);
body.tryAddImportToParentCompilationUnit(UncheckedIOException.class);

body.addStatement(StaticJavaParser.parseStatement("try { response.close(); }"
+ "catch (IOException e) { throw LOGGER.logThrowableAsError(new UncheckedIOException(e)); }"));
body.addStatement(StaticJavaParser.parseStatement("response.close();"));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,9 @@ public interface Response<T> extends Closeable {
static <T> Response<T> create(HttpRequest request, int statusCode, HttpHeaders headers, T value) {
return new HttpResponse<>(request, statusCode, headers, value);
}

/**
* Closes the response and releases any resources associated with it.
*/
void close();
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import io.clientcore.core.http.models.Response;
import io.clientcore.core.models.binarydata.BinaryData;

import java.io.IOException;
import java.util.List;

/**
Expand Down Expand Up @@ -162,7 +161,7 @@ public BinaryData getBody() {
* {@inheritDoc}
*/
@Override
public void close() throws IOException {
public void close() {
if (body != null) {
body.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ public BinaryData getBody() {
}

@Override
public void close() throws IOException {
public void close() {
if (bufferedBody == null) {
getBody();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
import io.clientcore.core.instrumentation.logging.ClientLogger;
import io.clientcore.core.instrumentation.logging.LoggingEvent;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.HttpURLConnection;
import java.net.URI;
import java.util.Collections;
Expand Down Expand Up @@ -177,11 +175,7 @@ private void createRedirectRequest(Response<?> redirectResponse) {
redirectResponse.getRequest().getHeaders().remove(HttpHeaderName.AUTHORIZATION);
redirectResponse.getRequest().setUri(redirectResponse.getHeaders().getValue(this.locationHeader));

try {
redirectResponse.close();
} catch (IOException e) {
throw LOGGER.logThrowableAsError(new UncheckedIOException(e));
}
redirectResponse.close();
}

private void logRedirect(ClientLogger logger, boolean lastAttempt, String redirectUri, int tryCount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import io.clientcore.core.utils.configuration.Configuration;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.HttpURLConnection;
import java.time.DateTimeException;
import java.time.Duration;
Expand Down Expand Up @@ -210,12 +209,7 @@ private Response<?> attempt(final HttpRequest httpRequest, final HttpPipelineNex
final Duration delayDuration = determineDelayDuration(response, tryCount, delayFromHeaders);

logRetry(logger.atVerbose(), tryCount, delayDuration, null, false, instrumentationContext);

try {
response.close();
} catch (IOException e) {
throw LOGGER.logThrowableAsError(new UncheckedIOException(e));
}
response.close();

long millis = delayDuration.toMillis();
if (millis > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import io.clientcore.core.http.models.Response;
import io.clientcore.core.instrumentation.logging.ClientLogger;

import java.io.IOException;
import java.util.Objects;

/**
Expand Down Expand Up @@ -82,11 +81,7 @@ public Response<?> process(HttpRequest httpRequest, HttpPipelineNextPolicy next)
if (httpResponse.getStatusCode() == 401 && authHeader != null) {
if (authorizeRequestOnChallenge(httpRequest, httpResponse)) {
// body needs to be closed or read to the end to release the connection
try {
httpResponse.close();
} catch (IOException e) {
throw LOGGER.logThrowableAsError(new RuntimeException(e));
}
httpResponse.close();
return nextPolicy.process();
} else {
return httpResponse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import io.clientcore.core.http.models.Response;
import io.clientcore.core.models.binarydata.BinaryData;

import java.io.IOException;
import java.util.function.Function;

/**
Expand Down Expand Up @@ -165,7 +164,7 @@ private HttpResponse<T> setBodyDeserializer(Function<BinaryData, Object> bodyDes
}

@Override
public void close() throws IOException {
public void close() {
BinaryData body = getBody();
if (body != null) {
body.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,7 @@ private static Object handleRestResponseReturnType(Response<?> response, Swagger
final Type bodyType = TypeUtil.getRestResponseBodyType(entityType);

if (TypeUtil.isTypeOrSubTypeOf(bodyType, Void.class)) {
try {
response.close();
} catch (IOException e) {
throw LOGGER.logThrowableAsError(new UncheckedIOException(e));
}
response.close();

return createResponseIfNecessary(response, entityType, null);
} else {
Expand Down Expand Up @@ -461,11 +457,7 @@ private static Object handleRestReturnType(Response<?> response, SwaggerMethodPa
final Object result;

if (TypeUtil.isTypeOrSubTypeOf(returnType, void.class) || TypeUtil.isTypeOrSubTypeOf(returnType, Void.class)) {
try {
expectedResponse.close();
} catch (IOException e) {
throw LOGGER.logThrowableAsError(new UncheckedIOException(e));
}
expectedResponse.close();

result = null;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -808,4 +808,9 @@ public void writeTo(WritableByteChannel channel) throws IOException {
public static BinaryData empty() {
return BinaryData.EMPTY;
}

/**
* Closes the underlying resource of this {@link BinaryData} if it is closeable.
*/
public abstract void close();
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public BinaryData toReplayableBinaryData() {
}

@Override
public void close() throws IOException {
public void close() {
// no-op
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ private byte[] getBytes() {
}

@Override
public void close() throws IOException {
public void close() {
// no-op
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ private byte[] getBytes() {
}

@Override
public void close() throws IOException {
public void close() {
// Since this uses a Path, there is nothing to close, therefore no-op.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ private byte[] getBytes() {
}

@Override
public void close() throws IOException {
content.get().close();
public void close() {
try {
content.get().close();
} catch (IOException e) {
throw LOGGER.logThrowableAsError(new UncheckedIOException(e));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private byte[] getBytes() {
}

@Override
public void close() throws IOException {
public void close() {
// no-op
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ private byte[] getBytes() {
}

@Override
public void close() throws IOException {
public void close() {
// no-op
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private byte[] getBytes() {
}

@Override
public void close() throws IOException {
public void close() {
// no-op
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.junit.jupiter.params.provider.MethodSource;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.nio.file.Files;
Expand Down Expand Up @@ -101,7 +100,7 @@ Response<List<Foo>> listNextFoo(@PathParam(value = "nextLink", encoded = true) S
}

@Test
public void contentTypeHeaderPriorityOverBodyParamAnnotationTest() throws IOException {
public void contentTypeHeaderPriorityOverBodyParamAnnotationTest() {
HttpClient client = new LocalHttpClient();
HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(client).build();

Expand Down Expand Up @@ -222,7 +221,7 @@ public Response<?> send(HttpRequest request) {

return new MockHttpResponse(request, success ? 200 : 400) {
@Override
public void close() throws IOException {
public void close() {
closeCalledOnResponse = true;

super.close();
Expand All @@ -236,7 +235,7 @@ public HttpRequest getLastHttpRequest() {
}

@Test
public void doesNotChangeEncodedPath() throws IOException {
public void doesNotChangeEncodedPath() {
String nextLinkUri
= "https://management.somecloud.com:443/subscriptions/000/resourceGroups/rg/providers/Microsoft.Compute/virtualMachineScaleSets/vmss1/virtualMachines?api-version=2021-11-01&$skiptoken=Mzk4YzFjMzMtM2IwMC00OWViLWI2NGYtNjg4ZTRmZGQ1Nzc2IS9TdWJzY3JpcHRpb25zL2VjMGFhNWY3LTllNzgtNDBjOS04NWNkLTUzNWM2MzA1YjM4MC9SZXNvdXJjZUdyb3Vwcy9SRy1XRUlEWFUtVk1TUy9WTVNjYWxlU2V0cy9WTVNTMS9WTXMvNzc=";
HttpPipeline pipeline = new HttpPipelineBuilder().httpClient((request) -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void constructor() throws IOException {
}

@Test
public void constructorWithNullValue() throws IOException {
public void constructorWithNullValue() {
try (HttpResponse<?> response = new HttpResponse<>(HTTP_REQUEST, STATUS_CODE, HEADERS, null)) {
assertEquals(HTTP_REQUEST, response.getRequest());
assertEquals(STATUS_CODE, response.getStatusCode());
Expand All @@ -54,7 +54,7 @@ public void constructorWithNullValue() throws IOException {
}

@Test
public void constructorWithNullValueAndBodySet() throws IOException {
public void constructorWithNullValueAndBodySet() {
try (HttpResponse<?> response = new HttpResponse<>(HTTP_REQUEST, STATUS_CODE, HEADERS, null)) {
assertEquals(HTTP_REQUEST, response.getRequest());
assertEquals(STATUS_CODE, response.getStatusCode());
Expand All @@ -69,7 +69,7 @@ public void constructorWithNullValueAndBodySet() throws IOException {
}

@Test
public void constructorWithNullValueBodySetAndBodyDeserializer() throws IOException {
public void constructorWithNullValueBodySetAndBodyDeserializer() {
try (HttpResponse<?> response = new HttpResponse<>(HTTP_REQUEST, STATUS_CODE, HEADERS, null)) {
assertEquals(HTTP_REQUEST, response.getRequest());
assertEquals(STATUS_CODE, response.getStatusCode());
Expand All @@ -92,7 +92,7 @@ public void constructorWithNullValueBodySetAndBodyDeserializer() throws IOExcept
}

@Test
public void constructorWithValueDoesNotDeserializeBody() throws IOException {
public void constructorWithValueDoesNotDeserializeBody() {
try (HttpResponse<?> response = new HttpResponse<>(HTTP_REQUEST, STATUS_CODE, HEADERS, VALUE)) {
assertEquals(HTTP_REQUEST, response.getRequest());
assertEquals(STATUS_CODE, response.getStatusCode());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.util.Collections;
import java.util.List;

Expand Down Expand Up @@ -49,8 +48,6 @@ public void testPagedResponseRequired() {
Assertions.assertEquals(firstLink, pagedResponse.getFirstLink());
Assertions.assertEquals(lastLink, pagedResponse.getLastLink());
}
} catch (IOException e) {
Assertions.fail();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@
import io.clientcore.core.http.models.Response;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class HttpPipelinePolicyTests {
@Test
public void verifySend() throws IOException {
public void verifySend() {
SyncPolicy policy1 = new SyncPolicy();
SyncPolicy policy2 = new SyncPolicy();

Expand All @@ -33,7 +32,7 @@ public void verifySend() throws IOException {
}

@Test
public void defaultImplementationShouldCallRightStack() throws IOException {
public void defaultImplementationShouldCallRightStack() {
DefaultImplementationSyncPolicy policyWithDefaultSyncImplementation = new DefaultImplementationSyncPolicy();

HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(new NoOpHttpClient())
Expand All @@ -50,7 +49,7 @@ public void defaultImplementationShouldCallRightStack() throws IOException {
* This is to cover case when reactor could complain about blocking on non-blocking thread.
*/
@Test
public void doesNotThrowThatThreadIsNonBlocking() throws IOException {
public void doesNotThrowThatThreadIsNonBlocking() {
SyncPolicy policy1 = new SyncPolicy();
HttpPipelinePolicy badPolicy1 = (ignored, next) -> {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,11 @@ public Response<?> send(HttpRequest request) {
}

@Test
public void retryConsumesBody() throws IOException {
public void retryConsumesBody() {
AtomicInteger closeCalls = new AtomicInteger();
Response<?> closeTrackingHttpResponse = new MockHttpResponse(null, 503, new HttpHeaders()) {
@Override
public void close() throws IOException {
public void close() {
closeCalls.incrementAndGet();
super.close();
}
Expand Down Expand Up @@ -278,7 +278,7 @@ public void propagatingExceptionHasOtherErrorsAsSuppressedExceptions() {

@ParameterizedTest
@MethodSource("getWellKnownRetryDelaySupplier")
public void retryWellKnownRetryHeaders(HttpHeaders responseHeaders) throws IOException {
public void retryWellKnownRetryHeaders(HttpHeaders responseHeaders) {
HttpRetryOptions retryOptions = new HttpRetryOptions(1, Duration.ofMillis(1));

AtomicInteger attemptCount = new AtomicInteger();
Expand Down
Loading
Loading