Skip to content

chore: remove empty catch block #1113

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.

Sign up for GitHub

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

Merged
antusus merged 2 commits into from
Oct 17, 2022
Merged

chore: remove empty catch block #1113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5dbc657
chore: removing some todos
antusus Oct 11, 2022
ee5532f
chore: removing empty catch blocks
antusus Oct 12, 2022
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
10 changes: 0 additions & 10 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,6 @@ task integrationTest(type: Test) {
classpath = sourceSets.intTest.runtimeClasspath
}

//TODO: enable JWT tests
//task integrationTestJWT(type: Test) {
// description "Runs the JWT based integration tests."
// group "Verification"
// testLogging.showStandardStreams = true
// useJUnit {
// includeCategories "com.box.sdk.IntegrationTestJWT"
// }
//}

jacoco {
reportsDirectory = file("$buildDir/reports/jacoco")
}
Expand Down
34 changes: 17 additions & 17 deletions src/main/java/com/box/sdk/BoxAPIRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,6 @@ protected BoxAPIRequest(BoxAPIConnection api, URL url, String method, String med
this.readTimeout = BoxGlobalSettings.getReadTimeout();
}

//TODO: should I add this? Breaks tests as client expets to get GZIP response
// this.addHeader("Accept-Encoding", "gzip");
this.addHeader("Accept-Charset", "utf-8");
}

Expand Down Expand Up @@ -146,28 +144,30 @@ public static boolean isRequestRetryable(BoxAPIException apiException) {
* @return true if the response is one that should be retried, otherwise false
*/
public static boolean isResponseRetryable(int responseCode, BoxAPIException apiException) {
if (responseCode >= 500 || responseCode == 429) {
return true;
}
return isClockSkewError(responseCode, apiException);
}

private static boolean isClockSkewError(int responseCode, BoxAPIException apiException) {
String response = apiException.getResponse();
if (response == null || response.length() == 0) {
return false;
}
String message = apiException.getMessage();
String errorCode = "";

//TODO: remove this empty catch and fix failing tests
try {
JsonObject responseBody = Json.parse(response).asObject();
if (responseBody.get("code") != null) {
errorCode = responseBody.get("code").toString();
} else if (responseBody.get("error") != null) {
errorCode = responseBody.get("error").toString();
}
} catch (Exception e) {
JsonObject responseBody = Json.parse(response).asObject();
if (responseBody.get("code") != null) {
errorCode = responseBody.get("code").toString();
} else if (responseBody.get("error") != null) {
errorCode = responseBody.get("error").toString();
}

boolean isClockSkewError = responseCode == 400
return responseCode == 400
&& errorCode.contains("invalid_grant")
&& message.contains("exp");

return (isClockSkewError
|| responseCode >= 500
|| responseCode == 429);
}

private static boolean isResponseRedirect(int responseCode) {
Expand Down Expand Up @@ -459,7 +459,6 @@ BoxFileUploadSessionPart sendForUploadPart(BoxFileUploadSession session, long of
throw apiException;
}
if (apiException.getResponseCode() == 500) {
//TODO: another empty catch block...
try {
Iterable<BoxFileUploadSessionPart> parts = session.listParts();
for (BoxFileUploadSessionPart part : parts) {
Expand All @@ -468,6 +467,7 @@ BoxFileUploadSessionPart sendForUploadPart(BoxFileUploadSession session, long of
}
}
} catch (BoxAPIException e) {
// ignoring exception as we are retrying
}
}
LOGGER.warn(format(
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/box/sdk/BoxAPIResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ public BoxAPIResponse(int code,
this.logResponse();
} else {
this.logErrorResponse(this.responseCode);
//TODO: log body when error occurs
throw new BoxAPIResponseException("The API returned an error code", responseCode, null, headers);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/com/box/sdk/BoxAPIRequestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class BoxAPIRequestTest {

@Test
public void requestRetriesTheDefaultNumberOfTimesWhenServerReturns500() {
stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500)));
stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500).withBody("{}")));
Time mockTime = mock(Time.class);
BackoffCounter backoffCounter = new BackoffCounter(mockTime);

Expand All @@ -50,7 +50,7 @@ public void requestRetriesTheDefaultNumberOfTimesWhenServerReturns500() {

@Test
public void requestRetriesTheDefaultNumberOfTimesWhenServerReturns429() {
stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(429)));
stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(429).withBody("{}")));
Time mockTime = mock(Time.class);
BackoffCounter backoffCounter = new BackoffCounter(mockTime);

Expand Down Expand Up @@ -87,7 +87,7 @@ public void requestRetriesTheDefaultNumberOfTimesWhenServerReturnsInvalidGrantIn
@Test
public void requestRetriesTheNumberOfTimesConfiguredInTheAPIConnection() {
final int expectedNumRetryAttempts = 1;
stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500)));
stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500).withBody("{}")));
Time mockTime = mock(Time.class);
BackoffCounter backoffCounter = new BackoffCounter(mockTime);

Expand Down
4 changes: 3 additions & 1 deletion src/test/java/com/box/sdk/BoxFileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,7 @@ public void testSetClassification() {

wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL))
.willReturn(WireMock.aResponse()
.withBody("{}")
.withStatus(409)));

wireMockRule.stubFor(WireMock.put(WireMock.urlPathEqualTo(metadataURL))
Expand All @@ -704,6 +705,7 @@ public void testSetClassificationThrowsException() {

wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL))
.willReturn(WireMock.aResponse()
.withBody("{}")
.withStatus(403)));

BoxFile file = new BoxFile(this.api, fileID);
Expand Down Expand Up @@ -1038,7 +1040,7 @@ public void setMetadataWorksWhenNoChangesSubmittedAndConflictOccured() {
request -> {
if (request.getMethod().equals("POST")) {
postCounter.incrementAndGet();
throw new BoxAPIException("Conflict", 409, "Conflict");
throw new BoxAPIException("Conflict", 409, "{}");
}
if (request.getMethod().equals("GET")) {
getCounter.incrementAndGet();
Expand Down
12 changes: 8 additions & 4 deletions src/test/java/com/box/sdk/BoxFolderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,17 +466,19 @@ public void testGetAllRootFolderItemsSucceeds() {
final String modifiedByLogin = "test@user.com";
final String modifiedByName = "Test User";

String result = TestUtils.getFixture("BoxFolder/GetAllRootFolderItems200");
String items = TestUtils.getFixture("BoxFolder/GetAllRootFolderItems200");
String info = TestUtils.getFixture("BoxFolder/GetFolderInfo200");

wireMockRule.stubFor(WireMock.get(WireMock.urlPathEqualTo(rootFolderItemsURL))
.willReturn(WireMock.aResponse()
.withHeader("Content-Type", APPLICATION_JSON)
.withStatus(200)));
.withStatus(200).withBody(items)));

wireMockRule.stubFor(WireMock.get(WireMock.urlPathEqualTo(folderURL))
.willReturn(WireMock.aResponse()
.withHeader("Content-Type", APPLICATION_JSON)
.withBody(result)));
.withBody(info)
.withStatus(200)));

BoxFolder rootFolder = BoxFolder.getRootFolder(this.api);
BoxFolder.Info rootFolderInfo = rootFolder.getInfo();
Expand Down Expand Up @@ -1133,6 +1135,7 @@ public void testSetClassification() {

wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL))
.willReturn(WireMock.aResponse()
.withBody("{}")
.withStatus(409)));

wireMockRule.stubFor(WireMock.put(WireMock.urlPathEqualTo(metadataURL))
Expand All @@ -1156,6 +1159,7 @@ public void testSetClassificationThrowsException() {

wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL))
.willReturn(WireMock.aResponse()
.withBody("{}")
.withStatus(403)));

BoxFolder folder = new BoxFolder(this.api, folderID);
Expand Down Expand Up @@ -1524,7 +1528,7 @@ public void setMetadataWorksWhenNoChangesSubmittedAndConflictOccured() {
request -> {
if (request.getMethod().equals("POST")) {
postCounter.incrementAndGet();
throw new BoxAPIException("Conflict", 409, "Conflict");
throw new BoxAPIException("Conflict", 409, "{}");
}
if (request.getMethod().equals("GET")) {
getCounter.incrementAndGet();
Expand Down