[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions

["Hadoop QA (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15883718#comment-15883718 ] 

Hadoop QA commented on SENTRY-1546:
-----------------------------------

Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12854601/SENTRY-1546.003.patch against master.

{color:red}Overall:{color} -1 due to 2 errors

{color:red}ERROR:{color} mvn test exited 1
{color:red}ERROR:{color} Failed: org.apache.sentry.provider.db.generic.service.persistent.TestPrivilegeOperatePersistence

Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/2368/console

This message is automatically generated.

> 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, SENTRY-1546.003.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.15#6346)