[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15826848#comment-15826848
]
Alexander Kolbasov commented on SENTRY-1546:
--------------------------------------------
Since we want a unified message in such cases would it make sense to modify
constructors for SentryNoSuchObjectException and SentryAlreadyExistsException
to provide such uniform messages for us so that callers don't have to hunt for
proper message format?
> Generic Policy provides bad error messages for Sentry exceptions
> ----------------------------------------------------------------
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
> Affects Versions: 1.8.0
> Reporter: Alexander Kolbasov
> Assignee: kalyan kumar kalvagadda
> Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch
>
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response<Void> respose = requestHandle(new RequestHandler<Void>() {
> @Override
> public Response<Void> handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response<Void>(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
