make planner errors be user persona (#17437)

Change the persona for errors within the planner from Admin to User. The ADMIN persona is meant to be "a persona who is interacting with admin APIs and understands Druid query concepts". This isn't an admin API, it's a query API. Low quality error messages being returned to the correct audience is better than hiding all error messages.

The errors that can be returned back can be user solvable, and other times requires a druid expert. But the errors do not leak information that should only be seen by more expert/privileged personas.

The original ADMIN persona showed some reticence to tag low-quality error messages with a USER persona. but it really does seem user-directed to me so USER to me would make sense.
This commit is contained in:
Tom 2024-11-04 10:48:35 -08:00 committed by GitHub
parent 5fcf4205e4
commit e4cdbca23c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 8 additions and 7 deletions

View File

@ -511,7 +511,7 @@ public class MSQFaultsTest extends MSQTestBase
+ "PARTITIONED BY ALL TIME")
.setExpectedValidationErrorMatcher(
new DruidExceptionMatcher(
DruidException.Persona.ADMIN,
DruidException.Persona.USER,
DruidException.Category.INVALID_INPUT,
"general"
).expectMessageContains(
@ -533,7 +533,7 @@ public class MSQFaultsTest extends MSQTestBase
)
.setExpectedValidationErrorMatcher(
new DruidExceptionMatcher(
DruidException.Persona.ADMIN,
DruidException.Persona.USER,
DruidException.Category.INVALID_INPUT,
"general"
).expectMessageContains(

View File

@ -678,9 +678,10 @@ public abstract class QueryHandler extends SqlStatementHandler.BaseStatementHand
.build(exception, "Unhandled Query Planning Failure, see broker logs for details");
} else {
// Planning errors are more like hints: it isn't guaranteed that the planning error is actually what went wrong.
// For this reason, we consider these as targetting a more expert persona, i.e. the admin instead of the actual
// user.
throw DruidException.forPersona(DruidException.Persona.ADMIN)
// Even though the errors could be targetted to a more expert persona the errors aren't leaking any privileged
// information about the cluster that an admin might care about. The errors that are user resolvable are worth
// the potential confusion that a user might face with one that requires an expert persona.
throw DruidException.forPersona(DruidException.Persona.USER)
.ofCategory(DruidException.Category.INVALID_INPUT)
.build(
exception,

View File

@ -690,7 +690,7 @@ public class BaseCalciteQueryTest extends CalciteTestBase
if (testBuilder().isDecoupledMode()) {
return new DruidExceptionMatcher(Persona.USER, Category.INVALID_INPUT, "invalidInput");
} else {
return new DruidExceptionMatcher(Persona.ADMIN, Category.INVALID_INPUT, "general");
return new DruidExceptionMatcher(Persona.USER, Category.INVALID_INPUT, "general");
}
}

View File

@ -1386,7 +1386,7 @@ public class SqlResourceTest extends CalciteTestBase
validateErrorResponse(
exception,
"general",
DruidException.Persona.ADMIN,
DruidException.Persona.USER,
DruidException.Category.INVALID_INPUT,
"Query could not be planned. A possible reason is "
+ "[SQL query requires ordering a table by non-time column [[dim1]], which is not supported.]"