Skip to content

Commit

Permalink
Add getStatusCode into WebClientWithMessageException (#176)
Browse files Browse the repository at this point in the history
## Summary

<!--- HINT: Replace #nnn with corresponding Issue number, if you are
fixing an existing issue -->

Add support for `getStatusCode()` in `WebClientWithMessageException` so
we can expose status code information to users without exposing
`HttpStatus`, so that callers can understand how to handle the
exception. Discussed in [this
PR](#172).

## Changes

- [ ] Client-facing API Changes
- [ ] Internal API Changes
- [ ] Bug Fixes
- [ ] New Features
- [ ] Performance Improvements
- [ ] Code Style
- [x] Refactoring
- [ ] Documentation
- [ ] Tests

For all the boxes checked, please include additional details of the
changes made in this pull request.

## Testing Done
<!--- Check any relevant boxes with "x" -->

- [ ] Manually Tested on local docker setup. Please include commands
ran, and their output.
- [ ] Added new tests for the changes made.
- [x] Updated existing tests to reflect the changes made.
- [ ] No tests added or updated. Please explain why. If unsure, please
feel free to ask for help.
- [ ] Some other form of testing like staging or soak time in
production. Please explain.

For all the boxes checked, include a detailed description of the testing
done for the changes made in this pull request.

# Additional Information

- [ ] Breaking Changes
- [ ] Deprecations
- [ ] Large PR broken into smaller PRs, and PR plan linked in the
description.

For all the boxes checked, include additional details of the changes
made in this pull request.
  • Loading branch information
chenselena authored Aug 27, 2024
1 parent b302209 commit 28042d9
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,8 @@ public String getMessage() {
: String.format(
"%s , %s", responseException.getMessage(), responseException.getResponseBodyAsString());
}

public int getStatusCode() {
return responseException.getRawStatusCode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
import static com.linkedin.openhouse.spark.SparkTestBase.mockTableService;
import static com.linkedin.openhouse.spark.SparkTestBase.spark;

import com.linkedin.openhouse.javaclient.exception.WebClientWithMessageException;
import com.linkedin.openhouse.javaclient.exception.WebClientResponseWithMessageException;
import com.linkedin.openhouse.relocated.org.springframework.http.HttpStatus;
import com.linkedin.openhouse.spark.SparkTestBase;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
Expand Down Expand Up @@ -72,13 +73,14 @@ public void testAuthZFailure() {
mockResponse(
403,
"{\"status\":\"FORBIDDEN\",\"error\":\"forbidden\",\"message\":\"Operation on database dbCreate failed as user sraikar is unauthorized\"}"));
WebClientWithMessageException exception =
WebClientResponseWithMessageException exception =
Assertions.assertThrows(
WebClientWithMessageException.class,
WebClientResponseWithMessageException.class,
() -> spark.sql("CREATE TABLE openhouse.dbCreate.tbError (col1 string, col2 string)"));
Assertions.assertTrue(
exception
.getMessage()
.contains("Operation on database dbCreate failed as user sraikar is unauthorized"));
Assertions.assertEquals(exception.getStatusCode(), HttpStatus.FORBIDDEN.value());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import static com.linkedin.openhouse.spark.MockHelpers.*;
import static com.linkedin.openhouse.spark.SparkTestBase.*;

import com.linkedin.openhouse.javaclient.exception.WebClientWithMessageException;
import com.linkedin.openhouse.javaclient.exception.WebClientResponseWithMessageException;
import com.linkedin.openhouse.relocated.org.springframework.http.HttpStatus;
import com.linkedin.openhouse.spark.SparkTestBase;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.spark.sql.AnalysisException;
Expand Down Expand Up @@ -95,11 +96,13 @@ public void test503Error() {
mockResponse(
503,
"{\"status\":\"SERVICE_UNAVAILABLE\",\"error\":\"Service Unavailable\",\"message\":\"Drop table failed as service is unavailable\"}"));
WebClientWithMessageException exception =
WebClientResponseWithMessageException exception =
Assertions.assertThrows(
WebClientWithMessageException.class, () -> spark.sql("DROP TABLE openhouse.dbDrop.t2"));
WebClientResponseWithMessageException.class,
() -> spark.sql("DROP TABLE openhouse.dbDrop.t2"));
Assertions.assertTrue(
exception.getMessage().contains("\"Drop table failed as service is unavailable"));
Assertions.assertEquals(exception.getStatusCode(), HttpStatus.SERVICE_UNAVAILABLE.value());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
import static com.linkedin.openhouse.spark.SparkTestBase.*;

import com.google.common.collect.ImmutableList;
import com.linkedin.openhouse.javaclient.exception.WebClientWithMessageException;
import com.linkedin.openhouse.javaclient.exception.WebClientResponseWithMessageException;
import com.linkedin.openhouse.relocated.org.springframework.http.HttpStatus;
import com.linkedin.openhouse.spark.SparkTestBase;
import java.sql.Timestamp;
import java.util.List;
Expand Down Expand Up @@ -129,14 +130,15 @@ public void testAuthzError() {
mockResponse(
403,
"{\"status\":\"FORBIDDEN\",\"error\":\"forbidden\",\"message\":\"Operation on table dbSelect.errorTbl failed as user sraikar is unauthorized\"}"));
WebClientWithMessageException exception =
WebClientResponseWithMessageException exception =
Assertions.assertThrows(
WebClientWithMessageException.class,
WebClientResponseWithMessageException.class,
() -> spark.sql("SELECT * FROM openhouse.dbSelect.errorTbl"));
Assertions.assertTrue(
exception
.getMessage()
.contains(
"Operation on table dbSelect.errorTbl failed as user sraikar is unauthorized"));
Assertions.assertEquals(exception.getStatusCode(), HttpStatus.FORBIDDEN.value());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
import com.google.common.collect.ImmutableList;
import com.google.gson.Gson;
import com.linkedin.openhouse.gen.tables.client.model.UpdateAclPoliciesRequestBody;
import com.linkedin.openhouse.javaclient.exception.WebClientWithMessageException;
import com.linkedin.openhouse.javaclient.exception.WebClientResponseWithMessageException;
import com.linkedin.openhouse.relocated.org.springframework.http.HttpStatus;
import com.linkedin.openhouse.spark.SparkTestBase;
import java.util.List;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -170,29 +171,32 @@ public void testOtherServerError() {
403,
"{\"status\":\"FORBIDDEN\",\"error\":\"forbidden\",\"message\":\"Operation on table db.tb1 failed as user sraikar is unauthorized\"}"));
String ddlWithSchema = "GRANT SELECT ON TABLE openhouse.dgrant.t1 TO sraikar";
WebClientWithMessageException exception =
WebClientResponseWithMessageException exception =
Assertions.assertThrows(
WebClientWithMessageException.class, () -> spark.sql(ddlWithSchema));
WebClientResponseWithMessageException.class, () -> spark.sql(ddlWithSchema));
Assertions.assertTrue(
exception
.getMessage()
.contains("Operation on table db.tb1 failed as user sraikar is unauthorized"));
Assertions.assertEquals(exception.getStatusCode(), HttpStatus.FORBIDDEN.value());

mockTableService.enqueue(
mockResponse(
500,
"{\"status\":\"INTERNAL_SERVER_ERROR\",\"error\":\"Internal Server Error\",\"message\":\"Something went wrong on the server\"}"));
exception =
Assertions.assertThrows(
WebClientWithMessageException.class, () -> spark.sql(ddlWithSchema));
WebClientResponseWithMessageException.class, () -> spark.sql(ddlWithSchema));
Assertions.assertTrue(exception.getMessage().contains("Something went wrong on the server"));
Assertions.assertEquals(exception.getStatusCode(), HttpStatus.INTERNAL_SERVER_ERROR.value());

// Test empty response
mockTableService.enqueue(new MockResponse().setResponseCode(401));
exception =
Assertions.assertThrows(
WebClientWithMessageException.class, () -> spark.sql(ddlWithSchema));
WebClientResponseWithMessageException.class, () -> spark.sql(ddlWithSchema));
Assertions.assertTrue(exception.getMessage().equals("401 Unauthorized"));
Assertions.assertEquals(exception.getStatusCode(), HttpStatus.UNAUTHORIZED.value());
}

private Dispatcher assertDispatcher(UpdateAclPoliciesRequestBody expectedRequestBody) {
Expand Down

0 comments on commit 28042d9

Please sign in to comment.