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)