You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sentry.apache.org by "Alexander Kolbasov (JIRA)" <ji...@apache.org> on 2016/11/28 17:35:58 UTC

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

     [ https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alexander Kolbasov updated SENTRY-1546:
---------------------------------------
    Labels: bite-sized  (was: )

> 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
>            Priority: Minor
>              Labels: bite-sized
>
> 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)