[ https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15826889#comment-15826889 ]
kalyan kumar kalvagadda commented on SENTRY-1546: ------------------------------------------------- It makes sense. I'm not sure if it is feasible in all the cases. > 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)