diff --git a/core/trino-main/src/main/java/io/trino/dispatcher/QueuedStatementResource.java b/core/trino-main/src/main/java/io/trino/dispatcher/QueuedStatementResource.java index da11f9045755..5e17f913eb5a 100644 --- a/core/trino-main/src/main/java/io/trino/dispatcher/QueuedStatementResource.java +++ b/core/trino-main/src/main/java/io/trino/dispatcher/QueuedStatementResource.java @@ -33,6 +33,7 @@ import io.trino.execution.QueryState; import io.trino.server.DisconnectionAwareAsyncResponse; import io.trino.server.ExternalUriInfo; +import io.trino.server.GoneException; import io.trino.server.HttpRequestSessionContextFactory; import io.trino.server.ServerConfig; import io.trino.server.SessionContext; @@ -48,21 +49,22 @@ import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import jakarta.servlet.http.HttpServletRequest; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.BeanParam; import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; import jakarta.ws.rs.QueryParam; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.container.Suspended; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.Response.Status; import java.net.URI; import java.util.Optional; @@ -93,10 +95,6 @@ import static io.trino.server.security.ResourceSecurity.AccessType.PUBLIC; import static io.trino.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR; import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; -import static jakarta.ws.rs.core.MediaType.TEXT_PLAIN_TYPE; -import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; -import static jakarta.ws.rs.core.Response.Status.FORBIDDEN; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static java.util.Objects.requireNonNull; import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -164,7 +162,7 @@ public Response postStatement( @BeanParam ExternalUriInfo externalUriInfo) { if (isNullOrEmpty(statement)) { - throw badRequest(BAD_REQUEST, "SQL statement is empty"); + throw new BadRequestException("SQL statement is empty"); } Query query = registerQuery(statement, servletRequest, httpHeaders); @@ -177,7 +175,7 @@ private Query registerQuery(String statement, HttpServletRequest servletRequest, Optional remoteAddress = Optional.ofNullable(servletRequest.getRemoteAddr()); Optional identity = authenticatedIdentity(servletRequest); if (identity.flatMap(Identity::getPrincipal).map(InternalPrincipal.class::isInstance).orElse(false)) { - throw badRequest(FORBIDDEN, "Internal communication can not be used to start a query"); + throw new ForbiddenException("Internal communication can not be used to start a query"); } MultivaluedMap headers = httpHeaders.getRequestHeaders(); @@ -241,7 +239,7 @@ private Query getQuery(QueryId queryId, String slug, long token) { Query query = queryManager.getQuery(queryId); if (query == null || !query.getSlug().isValid(QUEUED_QUERY, slug, token)) { - throw badRequest(NOT_FOUND, "Query not found"); + throw new NotFoundException("Query not found"); } return query; } @@ -296,15 +294,6 @@ private static QueryResults createQueryResults( null); } - private static WebApplicationException badRequest(Status status, String message) - { - throw new WebApplicationException( - Response.status(status) - .type(TEXT_PLAIN_TYPE) - .entity(message) - .build()); - } - private static final class Query { private final String query; @@ -387,7 +376,7 @@ public QueryResults getQueryResults(long token, ExternalUriInfo externalUriInfo) long lastToken = this.lastToken.get(); // token should be the last token or the next token if (token != lastToken && token != lastToken + 1) { - throw new WebApplicationException(Response.Status.GONE); + throw new GoneException("Invalid token"); } // advance (or stay at) the token this.lastToken.compareAndSet(lastToken, token); @@ -402,9 +391,7 @@ public QueryResults getQueryResults(long token, ExternalUriInfo externalUriInfo) DispatchInfo dispatchInfo = dispatchManager.getDispatchInfo(queryId) // query should always be found, but it may have just been determined to be abandoned - .orElseThrow(() -> new WebApplicationException(Response - .status(NOT_FOUND) - .build())); + .orElseThrow(NotFoundException::new); return createQueryResults(token + 1, externalUriInfo, dispatchInfo); } diff --git a/core/trino-main/src/main/java/io/trino/server/GoneException.java b/core/trino-main/src/main/java/io/trino/server/GoneException.java new file mode 100644 index 000000000000..3685079b013a --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/server/GoneException.java @@ -0,0 +1,31 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.server; + +import jakarta.ws.rs.WebApplicationException; +import jakarta.ws.rs.core.Response; + +public class GoneException + extends WebApplicationException +{ + public GoneException(String message) + { + super(message, Response.Status.GONE); + } + + public GoneException() + { + super(Response.Status.GONE); + } +} diff --git a/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java b/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java index 5ae7a2e61fce..f422f9bdb587 100644 --- a/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java +++ b/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java @@ -37,12 +37,9 @@ import io.trino.sql.parser.SqlParser; import io.trino.transaction.TransactionId; import jakarta.servlet.http.HttpServletRequest; -import jakarta.ws.rs.WebApplicationException; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.core.HttpHeaders; -import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.MultivaluedMap; -import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.Response.Status; import java.net.URLDecoder; import java.util.Collection; @@ -94,14 +91,13 @@ public SessionContext createSessionContext( MultivaluedMap headers, Optional remoteAddress, Optional authenticatedIdentity) - throws WebApplicationException { ProtocolHeaders protocolHeaders; try { protocolHeaders = detectProtocol(alternateHeaderName, headers.keySet()); } catch (ProtocolDetectionException e) { - throw badRequest(e.getMessage()); + throw new BadRequestException(e.getMessage()); } Optional catalog = Optional.ofNullable(trimEmptyToNull(headers.getFirst(protocolHeaders.requestCatalog()))); Optional schema = Optional.ofNullable(trimEmptyToNull(headers.getFirst(protocolHeaders.requestSchema()))); @@ -145,7 +141,7 @@ case ParsedSessionPropertyName(Optional catalogName, String propertyName // catalog session properties cannot be validated until the transaction has started catalogSessionProperties.computeIfAbsent(catalogName.orElseThrow(), id -> new HashMap<>()).put(propertyName, propertyValue); } - default -> throw badRequest(format("Invalid %s header", protocolHeaders.requestSession())); + default -> throw new BadRequestException(format("Invalid %s header", protocolHeaders.requestSession())); } } requireNonNull(catalogSessionProperties, "catalogSessionProperties is null"); @@ -196,7 +192,7 @@ public Identity extractAuthorizedIdentity(Optional optionalAuthenticat protocolHeaders = detectProtocol(alternateHeaderName, headers.keySet()); } catch (ProtocolDetectionException e) { - throw badRequest(e.getMessage()); + throw new BadRequestException(e.getMessage()); } Identity identity = buildSessionIdentity(optionalAuthenticatedIdentity, protocolHeaders, headers); @@ -320,7 +316,7 @@ private static SelectedRole toSelectedRole(ProtocolHeaders protocolHeaders, Stri role = SelectedRole.valueOf(value); } catch (IllegalArgumentException e) { - throw badRequest(format("Invalid %s header", protocolHeaders.requestRole())); + throw new BadRequestException(format("Invalid %s header", protocolHeaders.requestRole())); } return role; } @@ -340,7 +336,7 @@ private static Map parseProperty(MultivaluedMap properties.put(nameValue.get(0), urlDecode(nameValue.get(1))); } catch (IllegalArgumentException e) { - throw badRequest(format("Invalid %s header: %s", headerName, e)); + throw new BadRequestException(format("Invalid %s header: %s", headerName, e)); } } return properties; @@ -374,10 +370,10 @@ private static ResourceEstimates parseResourceEstimate(ProtocolHeaders protocolH builder.setPeakMemory(DataSize.valueOf(value)); return; } - throw badRequest(format("Unsupported resource name %s", name)); + throw new BadRequestException(format("Unsupported resource name %s", name)); } catch (IllegalArgumentException e) { - throw badRequest(format("Unsupported format for resource estimate '%s': %s", value, e)); + throw new BadRequestException(format("Unsupported format for resource estimate '%s': %s", value, e)); } }); @@ -397,7 +393,7 @@ private static ParsedSessionPropertyName parseSessionPropertyName(String value) private static void assertRequest(boolean expression, String format, Object... args) { if (!expression) { - throw badRequest(format(format, args)); + throw new BadRequestException(format(format, args)); } } @@ -410,7 +406,7 @@ private Map parsePreparedStatementsHeaders(ProtocolHeaders proto statementName = urlDecode(key); } catch (IllegalArgumentException e) { - throw badRequest(format("Invalid %s header: %s", protocolHeaders.requestPreparedStatement(), e.getMessage())); + throw new BadRequestException(format("Invalid %s header: %s", protocolHeaders.requestPreparedStatement(), e.getMessage())); } String sqlString = preparedStatementEncoder.decodePreparedStatementFromHeader(value); @@ -420,7 +416,7 @@ private Map parsePreparedStatementsHeaders(ProtocolHeaders proto sqlParser.createStatement(sqlString); } catch (ParsingException e) { - throw badRequest(format("Invalid %s header: %s", protocolHeaders.requestPreparedStatement(), e.getMessage())); + throw new BadRequestException(format("Invalid %s header: %s", protocolHeaders.requestPreparedStatement(), e.getMessage())); } preparedStatements.put(statementName, sqlString); @@ -439,19 +435,10 @@ private static Optional parseTransactionId(String transactionId) return Optional.of(TransactionId.valueOf(transactionId)); } catch (Exception e) { - throw badRequest(e.getMessage()); + throw new BadRequestException(e.getMessage()); } } - private static WebApplicationException badRequest(String message) - { - throw new WebApplicationException(message, Response - .status(Status.BAD_REQUEST) - .type(MediaType.TEXT_PLAIN) - .entity(message) - .build()); - } - private static String trimEmptyToNull(String value) { return emptyToNull(nullToEmpty(value).trim()); diff --git a/core/trino-main/src/main/java/io/trino/server/QueryResource.java b/core/trino-main/src/main/java/io/trino/server/QueryResource.java index 497f380178be..1aef011787d4 100644 --- a/core/trino-main/src/main/java/io/trino/server/QueryResource.java +++ b/core/trino-main/src/main/java/io/trino/server/QueryResource.java @@ -97,7 +97,7 @@ public Response getQueryInfo(@PathParam("queryId") QueryId queryId, @QueryParam( Optional queryInfo = dispatchManager.getFullQueryInfo(queryId) .map(info -> pruned ? pruneQueryInfo(info, info.getVersion()) : info); if (queryInfo.isEmpty()) { - return Response.status(Status.GONE).build(); + throw new GoneException(); } try { checkCanViewQueryOwnedBy(sessionContextFactory.extractAuthorizedIdentity(servletRequest, httpHeaders), queryInfo.get().getSession().toIdentity(), accessControl); @@ -165,7 +165,7 @@ private Response failQuery(QueryId queryId, TrinoException queryException, HttpS throw new ForbiddenException(); } catch (NoSuchElementException e) { - return Response.status(Status.GONE).build(); + throw new GoneException(); } } } diff --git a/core/trino-main/src/main/java/io/trino/server/QueryStateInfoResource.java b/core/trino-main/src/main/java/io/trino/server/QueryStateInfoResource.java index 7c48362c393f..bbf16bcede64 100644 --- a/core/trino-main/src/main/java/io/trino/server/QueryStateInfoResource.java +++ b/core/trino-main/src/main/java/io/trino/server/QueryStateInfoResource.java @@ -24,11 +24,11 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; import jakarta.ws.rs.QueryParam; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; @@ -46,7 +46,6 @@ import static io.trino.server.QueryStateInfo.createQueryStateInfo; import static io.trino.server.QueryStateInfo.createQueuedQueryStateInfo; import static io.trino.server.security.ResourceSecurity.AccessType.AUTHENTICATED_USER; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static java.util.Objects.requireNonNull; @Path("/v1/queryState") @@ -108,7 +107,6 @@ private QueryStateInfo getQueryStateInfo(BasicQueryInfo queryInfo) @Path("{queryId}") @Produces(MediaType.APPLICATION_JSON) public QueryStateInfo getQueryStateInfo(@PathParam("queryId") String queryId, @Context HttpServletRequest servletRequest, @Context HttpHeaders httpHeaders) - throws WebApplicationException { try { BasicQueryInfo queryInfo = dispatchManager.getQueryInfo(new QueryId(queryId)); @@ -119,7 +117,7 @@ public QueryStateInfo getQueryStateInfo(@PathParam("queryId") String queryId, @C throw new ForbiddenException(); } catch (NoSuchElementException e) { - throw new WebApplicationException(NOT_FOUND); + throw new NotFoundException(); } } } diff --git a/core/trino-main/src/main/java/io/trino/server/ResourceGroupStateInfoResource.java b/core/trino-main/src/main/java/io/trino/server/ResourceGroupStateInfoResource.java index de7c802d6921..b3f743283467 100644 --- a/core/trino-main/src/main/java/io/trino/server/ResourceGroupStateInfoResource.java +++ b/core/trino-main/src/main/java/io/trino/server/ResourceGroupStateInfoResource.java @@ -19,10 +19,10 @@ import io.trino.spi.resourcegroups.ResourceGroupId; import jakarta.ws.rs.Encoded; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.MediaType; import java.net.URLDecoder; @@ -31,7 +31,6 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.server.security.ResourceSecurity.AccessType.MANAGEMENT_READ; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; @@ -59,9 +58,9 @@ public ResourceGroupInfo getQueryStateInfos(@PathParam("resourceGroupId") String Arrays.stream(resourceGroupIdString.split("/")) .map(ResourceGroupStateInfoResource::urlDecode) .collect(toImmutableList()))) - .orElseThrow(() -> new WebApplicationException(NOT_FOUND)); + .orElseThrow(NotFoundException::new); } - throw new WebApplicationException(NOT_FOUND); + throw new NotFoundException(); } private static String urlDecode(String value) diff --git a/core/trino-main/src/main/java/io/trino/server/ServerInfoResource.java b/core/trino-main/src/main/java/io/trino/server/ServerInfoResource.java index b7b68adde5d0..2064080b3506 100644 --- a/core/trino-main/src/main/java/io/trino/server/ServerInfoResource.java +++ b/core/trino-main/src/main/java/io/trino/server/ServerInfoResource.java @@ -19,13 +19,13 @@ import io.trino.client.ServerInfo; import io.trino.metadata.NodeState; import io.trino.server.security.ResourceSecurity; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.PUT; import jakarta.ws.rs.Path; import jakarta.ws.rs.Produces; -import jakarta.ws.rs.WebApplicationException; -import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import java.util.Optional; @@ -37,7 +37,6 @@ import static io.trino.server.security.ResourceSecurity.AccessType.PUBLIC; import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; import static jakarta.ws.rs.core.MediaType.TEXT_PLAIN; -import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -76,7 +75,6 @@ public ServerInfo getInfo() @Consumes(APPLICATION_JSON) @Produces(TEXT_PLAIN) public Response updateState(NodeState state) - throws WebApplicationException { requireNonNull(state, "state is null"); return switch (state) { @@ -84,11 +82,7 @@ public Response updateState(NodeState state) shutdownHandler.requestShutdown(); yield Response.ok().build(); } - case ACTIVE, INACTIVE -> throw new WebApplicationException(Response - .status(BAD_REQUEST) - .type(MediaType.TEXT_PLAIN) - .entity(format("Invalid state transition to %s", state)) - .build()); + case ACTIVE, INACTIVE -> throw new BadRequestException(format("Invalid state transition to %s", state)); }; } @@ -114,6 +108,6 @@ public Response getServerCoordinator() return Response.ok().build(); } // return 404 to allow load balancers to only send traffic to the coordinator - return Response.status(Response.Status.NOT_FOUND).build(); + throw new NotFoundException(); } } diff --git a/core/trino-main/src/main/java/io/trino/server/TaskResource.java b/core/trino-main/src/main/java/io/trino/server/TaskResource.java index 188b90c4092e..15e13563f12b 100644 --- a/core/trino-main/src/main/java/io/trino/server/TaskResource.java +++ b/core/trino-main/src/main/java/io/trino/server/TaskResource.java @@ -43,11 +43,13 @@ import jakarta.ws.rs.DefaultValue; import jakarta.ws.rs.GET; import jakarta.ws.rs.HeaderParam; +import jakarta.ws.rs.InternalServerErrorException; import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; import jakarta.ws.rs.QueryParam; +import jakarta.ws.rs.ServiceUnavailableException; import jakarta.ws.rs.container.AsyncResponse; import jakarta.ws.rs.container.Suspended; import jakarta.ws.rs.core.Context; @@ -418,10 +420,7 @@ private boolean failRequestIfInvalid(AsyncResponse asyncResponse) // Ideally the coordinator should not schedule tasks on worker that is not ready, but in pipelined execution there is currently no way to move a task. // Accepting a request too early will likely lead to some failure and HTTP 500 (INTERNAL_SERVER_ERROR) response. The coordinator won't retry on this. // Send 503 (SERVICE_UNAVAILABLE) so that request is retried. - asyncResponse.resume(Response.status(Status.SERVICE_UNAVAILABLE) - .type(MediaType.TEXT_PLAIN_TYPE) - .entity("The server is not fully started yet ") - .build()); + asyncResponse.resume(new ServiceUnavailableException("The server is not fully started yet")); return true; } return false; @@ -453,7 +452,7 @@ private boolean injectFailure( case TASK_MANAGEMENT_REQUEST_FAILURE: if (requestType.isTaskManagement()) { log.info("Failing %s request for task %s", requestType, taskId); - asyncResponse.resume(Response.serverError().build()); + asyncResponse.resume(new InternalServerErrorException("Task %s failed".formatted(taskId))); return true; } break; @@ -467,7 +466,7 @@ private boolean injectFailure( case TASK_GET_RESULTS_REQUEST_FAILURE: if (!requestType.isTaskManagement()) { log.info("Failing %s request for task %s", requestType, taskId); - asyncResponse.resume(Response.serverError().build()); + asyncResponse.resume(new InternalServerErrorException("Task %s failed".formatted(taskId))); return true; } break; diff --git a/core/trino-main/src/main/java/io/trino/server/ThrowableMapper.java b/core/trino-main/src/main/java/io/trino/server/ThrowableMapper.java index dbebebd62b0f..51fcea69d23a 100644 --- a/core/trino-main/src/main/java/io/trino/server/ThrowableMapper.java +++ b/core/trino-main/src/main/java/io/trino/server/ThrowableMapper.java @@ -17,6 +17,12 @@ import com.google.inject.Inject; import io.airlift.log.Logger; import jakarta.servlet.http.HttpServletRequest; +import jakarta.ws.rs.BadRequestException; +import jakarta.ws.rs.ForbiddenException; +import jakarta.ws.rs.InternalServerErrorException; +import jakarta.ws.rs.NotFoundException; +import jakarta.ws.rs.ServerErrorException; +import jakarta.ws.rs.ServiceUnavailableException; import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.Response; @@ -45,20 +51,59 @@ public ThrowableMapper(ServerConfig config) @Override public Response toResponse(Throwable throwable) { - if (throwable instanceof WebApplicationException) { - return ((WebApplicationException) throwable).getResponse(); - } + // In HTTP/2 status consists of the code alone, without the reason phrase which exists only in the HTTP/1. + // Airlift enabled RESPONSE_SET_STATUS_OVER_SEND_ERROR which means that for 4xx and 5xx status codes, + // HttpServletResponse.setStatus will be called instead of HttpServletResponse.sendError. + // + // HttpServletResponse.sendError is problematic, as usually it resets entity, response headers and provide error page + // which is then rendered by the container implementation (e.g. Jetty). The generated errors depend on the implementation + // of the container and may not be consistent across different versions. + // + // Another problem with HttpServletResponse.sendError is that if the ServletFilter is used, it can't access + // ServletRequest/ServletResponse objects as they can be recycled by Jetty after sendError was called. + // + // With RESPONSE_SET_STATUS_OVER_SEND_ERROR enabled, the jax-rs application controls the process of returning errors + // and ServletFilters can access the request/response objects. + return switch (throwable) { + case ForbiddenException forbiddenException -> plainTextError(Response.Status.FORBIDDEN) + .entity("Error 403 Forbidden: " + forbiddenException.getMessage()) + .build(); + case ServiceUnavailableException serviceUnavailableException -> plainTextError(Response.Status.SERVICE_UNAVAILABLE) + .entity("Error 503 Service Unavailable: " + serviceUnavailableException.getMessage()) + .build(); + case NotFoundException notFoundException -> plainTextError(Response.Status.NOT_FOUND) + .entity("Error 404 Not Found: " + notFoundException.getMessage()) + .build(); + case BadRequestException badRequestException -> plainTextError(Response.Status.BAD_REQUEST) + .entity("Error 400 Bad Request: " + badRequestException.getMessage()) + .build(); + case InternalServerErrorException internalServerErrorException -> plainTextError(Response.Status.INTERNAL_SERVER_ERROR) + .entity("Error 500 Internal Server Error: " + internalServerErrorException.getMessage()) + .build(); + case ServerErrorException serverErrorException -> plainTextError(Response.Status.INTERNAL_SERVER_ERROR) + .entity("Error 500 Internal Server Error: " + serverErrorException.getMessage()) + .build(); + case GoneException goneException -> plainTextError(Response.Status.GONE) + .entity("Error 410 Gone: " + goneException.getMessage()) + .build(); + case WebApplicationException webApplicationException -> webApplicationException.getResponse(); + default -> { + log.warn(throwable, "Request failed for %s", request.getRequestURI()); - log.warn(throwable, "Request failed for %s", request.getRequestURI()); + ResponseBuilder responseBuilder = plainTextError(Response.Status.INTERNAL_SERVER_ERROR); + if (includeExceptionInResponse) { + responseBuilder.entity(Throwables.getStackTraceAsString(throwable)); + } + else { + responseBuilder.entity("Exception processing request"); + } + yield responseBuilder.build(); + } + }; + } - ResponseBuilder responseBuilder = Response.serverError() - .header(CONTENT_TYPE, TEXT_PLAIN); - if (includeExceptionInResponse) { - responseBuilder.entity(Throwables.getStackTraceAsString(throwable)); - } - else { - responseBuilder.entity("Exception processing request"); - } - return responseBuilder.build(); + private static ResponseBuilder plainTextError(Response.Status status) + { + return Response.status(status).header(CONTENT_TYPE, TEXT_PLAIN); } } diff --git a/core/trino-main/src/main/java/io/trino/server/protocol/ExecutingStatementResource.java b/core/trino-main/src/main/java/io/trino/server/protocol/ExecutingStatementResource.java index aaad1c0e2686..df2da65ba037 100644 --- a/core/trino-main/src/main/java/io/trino/server/protocol/ExecutingStatementResource.java +++ b/core/trino-main/src/main/java/io/trino/server/protocol/ExecutingStatementResource.java @@ -37,11 +37,11 @@ import jakarta.ws.rs.BeanParam; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; import jakarta.ws.rs.QueryParam; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.container.Suspended; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; @@ -60,8 +60,6 @@ import static io.trino.server.DisconnectionAwareAsyncResponse.bindDisconnectionAwareAsyncResponse; import static io.trino.server.protocol.Slug.Context.EXECUTING_QUERY; import static io.trino.server.security.ResourceSecurity.AccessType.PUBLIC; -import static jakarta.ws.rs.core.MediaType.TEXT_PLAIN_TYPE; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor; @@ -172,7 +170,7 @@ protected Query getQuery(QueryId queryId, String slug, long token) Query query = queries.get(queryId); if (query != null) { if (!query.isSlugValid(slug, token)) { - throw queryNotFound(); + throw new NotFoundException("Query not found"); } return query; } @@ -184,11 +182,11 @@ protected Query getQuery(QueryId queryId, String slug, long token) session = queryManager.getQuerySession(queryId); querySlug = queryManager.getQuerySlug(queryId); if (!querySlug.isValid(EXECUTING_QUERY, slug, token)) { - throw queryNotFound(); + throw new NotFoundException("Query not found"); } } catch (NoSuchElementException e) { - throw queryNotFound(); + throw new NotFoundException("Query not found"); } query = queries.computeIfAbsent(queryId, id -> Query.create( @@ -291,7 +289,7 @@ public Response cancelQuery( Query query = queries.get(queryId); if (query != null) { if (!query.isSlugValid(slug, token)) { - throw queryNotFound(); + throw new NotFoundException("Query not found"); } query.cancel(); return Response.noContent().build(); @@ -300,13 +298,13 @@ public Response cancelQuery( // cancel the query execution directly instead of creating the statement client try { if (!queryManager.getQuerySlug(queryId).isValid(EXECUTING_QUERY, slug, token)) { - throw queryNotFound(); + throw new NotFoundException("Query not found"); } queryManager.cancelQuery(queryId); return Response.noContent().build(); } catch (NoSuchElementException e) { - throw queryNotFound(); + throw new NotFoundException("Query not found"); } } @@ -323,15 +321,6 @@ public void partialCancel( query.partialCancel(stage); } - private static WebApplicationException queryNotFound() - { - throw new WebApplicationException( - Response.status(NOT_FOUND) - .type(TEXT_PLAIN_TYPE) - .entity("Query not found") - .build()); - } - private static String urlEncode(String value) { return URLEncoder.encode(value, UTF_8); diff --git a/core/trino-main/src/main/java/io/trino/server/protocol/Query.java b/core/trino-main/src/main/java/io/trino/server/protocol/Query.java index 8e0519abdd25..eb9063c04612 100644 --- a/core/trino-main/src/main/java/io/trino/server/protocol/Query.java +++ b/core/trino-main/src/main/java/io/trino/server/protocol/Query.java @@ -45,6 +45,7 @@ import io.trino.memory.context.SimpleLocalMemoryContext; import io.trino.operator.DirectExchangeClientSupplier; import io.trino.server.ExternalUriInfo; +import io.trino.server.GoneException; import io.trino.server.ResultQueryInfo; import io.trino.spi.ErrorCode; import io.trino.spi.Page; @@ -55,8 +56,7 @@ import io.trino.spi.type.Type; import io.trino.transaction.TransactionId; import io.trino.util.Ciphers; -import jakarta.ws.rs.WebApplicationException; -import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.NotFoundException; import java.net.URI; import java.util.List; @@ -382,18 +382,18 @@ private synchronized Optional getCachedResult(long token) // if this is a result before the lastResult, the data is gone if (token < lastToken) { - throw new WebApplicationException(Response.Status.GONE); + throw new GoneException(); } // if this is a request for a result after the end of the stream, return not found if (nextToken.isEmpty()) { - throw new WebApplicationException(Response.Status.NOT_FOUND); + throw new NotFoundException(); } // if this is not a request for the next results, return not found if (token != nextToken.getAsLong()) { // unknown token - throw new WebApplicationException(Response.Status.NOT_FOUND); + throw new NotFoundException(); } return Optional.empty(); diff --git a/core/trino-main/src/main/java/io/trino/server/ui/NoWebUiAuthenticationFilter.java b/core/trino-main/src/main/java/io/trino/server/ui/NoWebUiAuthenticationFilter.java index 62fea8e9de37..0b1d15a10ffb 100644 --- a/core/trino-main/src/main/java/io/trino/server/ui/NoWebUiAuthenticationFilter.java +++ b/core/trino-main/src/main/java/io/trino/server/ui/NoWebUiAuthenticationFilter.java @@ -13,10 +13,8 @@ */ package io.trino.server.ui; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.container.ContainerRequestContext; -import jakarta.ws.rs.core.Response; - -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; public class NoWebUiAuthenticationFilter implements WebUiAuthenticationFilter @@ -24,6 +22,6 @@ public class NoWebUiAuthenticationFilter @Override public void filter(ContainerRequestContext request) { - request.abortWith(Response.status(NOT_FOUND).build()); + throw new NotFoundException(); } } diff --git a/core/trino-main/src/main/java/io/trino/server/ui/UiQueryResource.java b/core/trino-main/src/main/java/io/trino/server/ui/UiQueryResource.java index 68f341f6adbb..f84974a64d31 100644 --- a/core/trino-main/src/main/java/io/trino/server/ui/UiQueryResource.java +++ b/core/trino-main/src/main/java/io/trino/server/ui/UiQueryResource.java @@ -21,6 +21,7 @@ import io.trino.security.AccessControl; import io.trino.server.BasicQueryInfo; import io.trino.server.DisableHttpCache; +import io.trino.server.GoneException; import io.trino.server.HttpRequestSessionContextFactory; import io.trino.server.security.ResourceSecurity; import io.trino.spi.QueryId; @@ -102,7 +103,7 @@ public Response getQueryInfo(@PathParam("queryId") QueryId queryId, @Context Htt throw new ForbiddenException(); } } - return Response.status(Status.GONE).build(); + throw new GoneException(); } @ResourceSecurity(WEB_UI) @@ -143,7 +144,7 @@ private Response failQuery(QueryId queryId, TrinoException queryException, HttpS throw new ForbiddenException(); } catch (NoSuchElementException e) { - return Response.status(Status.GONE).build(); + throw new GoneException(); } } } diff --git a/core/trino-main/src/main/java/io/trino/server/ui/WebUiStaticResource.java b/core/trino-main/src/main/java/io/trino/server/ui/WebUiStaticResource.java index 200312ca6701..db286b6745cc 100644 --- a/core/trino-main/src/main/java/io/trino/server/ui/WebUiStaticResource.java +++ b/core/trino-main/src/main/java/io/trino/server/ui/WebUiStaticResource.java @@ -17,6 +17,7 @@ import io.trino.server.security.ResourceSecurity; import jakarta.ws.rs.BeanParam; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; @@ -27,7 +28,6 @@ import static io.trino.server.security.ResourceSecurity.AccessType.PUBLIC; import static io.trino.server.security.ResourceSecurity.AccessType.WEB_UI; import static io.trino.web.ui.WebUiResources.webUiResource; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; @Path("") public class WebUiStaticResource @@ -55,7 +55,7 @@ public Response postFile() // The "getFile" resource method matches all GET requests, and without a // resource for POST requests, a METHOD_NOT_ALLOWED error will be returned // instead of a NOT_FOUND error - return Response.status(NOT_FOUND).build(); + throw new NotFoundException(); } // asset files are always visible diff --git a/core/trino-main/src/main/java/io/trino/server/ui/WorkerResource.java b/core/trino-main/src/main/java/io/trino/server/ui/WorkerResource.java index df7e119e25c7..5fb017148de1 100644 --- a/core/trino-main/src/main/java/io/trino/server/ui/WorkerResource.java +++ b/core/trino-main/src/main/java/io/trino/server/ui/WorkerResource.java @@ -27,6 +27,7 @@ import io.trino.metadata.NodeState; import io.trino.security.AccessControl; import io.trino.server.ForWorkerInfo; +import io.trino.server.GoneException; import io.trino.server.HttpRequestSessionContextFactory; import io.trino.server.security.ResourceSecurity; import io.trino.spi.Node; @@ -35,13 +36,12 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.Response.Status; import java.io.IOException; import java.util.HashSet; @@ -59,7 +59,6 @@ import static io.trino.server.security.ResourceSecurity.AccessType.WEB_UI; import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON_TYPE; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static java.util.Objects.requireNonNull; @Path("/ui/api/worker") @@ -122,7 +121,7 @@ public Response getThreads( throw new ForbiddenException(); } } - return Response.status(Status.GONE).build(); + throw new GoneException(); } @ResourceSecurity(WEB_UI) @@ -202,7 +201,7 @@ private Response proxyJsonResponse(String nodeId, String workerPath) InternalNode node = nodes.stream() .filter(n -> n.getNodeIdentifier().equals(nodeId)) .findFirst() - .orElseThrow(() -> new WebApplicationException(NOT_FOUND)); + .orElseThrow(NotFoundException::new); Request request = prepareGet() .setUri(uriBuilderFrom(node.getInternalUri()) diff --git a/core/trino-web-ui/src/main/java/io/trino/web/ui/WebUiResources.java b/core/trino-web-ui/src/main/java/io/trino/web/ui/WebUiResources.java index 0d58d4282b32..a6b55e6ab065 100644 --- a/core/trino-web-ui/src/main/java/io/trino/web/ui/WebUiResources.java +++ b/core/trino-web-ui/src/main/java/io/trino/web/ui/WebUiResources.java @@ -15,6 +15,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.io.Resources; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; import java.io.IOException; @@ -23,7 +24,6 @@ import java.net.URL; import java.nio.charset.StandardCharsets; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static java.util.Locale.ENGLISH; public class WebUiResources @@ -37,7 +37,7 @@ public static Response webUiResource(String path) { if (!isCanonical(path)) { // consider redirecting to the absolute path - return Response.status(NOT_FOUND).build(); + throw new NotFoundException(); } try { @@ -45,7 +45,7 @@ public static Response webUiResource(String path) return Response.ok(resource.openStream(), mediaType(resource.toString())).build(); } catch (IllegalArgumentException e) { - return Response.status(NOT_FOUND).build(); + throw new NotFoundException(); } } diff --git a/core/trino-web-ui/src/test/java/io/trino/web/ui/TestWebUiResources.java b/core/trino-web-ui/src/test/java/io/trino/web/ui/TestWebUiResources.java index 786d2406f6c8..f5b475013c58 100644 --- a/core/trino-web-ui/src/test/java/io/trino/web/ui/TestWebUiResources.java +++ b/core/trino-web-ui/src/test/java/io/trino/web/ui/TestWebUiResources.java @@ -13,6 +13,7 @@ */ package io.trino.web.ui; +import jakarta.ws.rs.NotFoundException; import org.junit.jupiter.api.Test; import java.io.IOException; @@ -20,6 +21,7 @@ import static io.trino.web.ui.WebUiResources.mediaType; import static io.trino.web.ui.WebUiResources.webUiResource; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class TestWebUiResources { @@ -27,8 +29,10 @@ class TestWebUiResources public void testUiResources() throws IOException { - assertThat(webUiResource("notExistingPath").getStatus()).isEqualTo(404); - assertThat(webUiResource("/webapp/index.html/../index.html").getStatus()).isEqualTo(404); // not canonical + assertThatThrownBy(() -> webUiResource("notExistingPath").getStatus()) + .isInstanceOf(NotFoundException.class); + assertThatThrownBy(() -> webUiResource("/webapp/index.html/../index.html").getStatus()) + .isInstanceOf(NotFoundException.class); // not canonical assertThat(webUiResource("/webapp/index.html").getStatus()).isEqualTo(200); assertThat(webUiResource("/webapp-preview/dist/index.html").getStatus()).isEqualTo(200); diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/backup/TestingHttpBackupResource.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/backup/TestingHttpBackupResource.java index 17eb2634d2e1..0d6a237f7f8b 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/backup/TestingHttpBackupResource.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/backup/TestingHttpBackupResource.java @@ -17,17 +17,20 @@ import com.google.inject.Inject; import io.airlift.slice.Slices; import io.airlift.slice.XxHash64; +import io.trino.server.GoneException; import io.trino.spi.NodeManager; import jakarta.servlet.http.HttpServletRequest; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.GET; import jakarta.ws.rs.HEAD; import jakarta.ws.rs.HeaderParam; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.PUT; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.Response; @@ -39,10 +42,6 @@ import static io.trino.plugin.raptor.legacy.backup.HttpBackupStore.CONTENT_XXH64; import static io.trino.plugin.raptor.legacy.backup.HttpBackupStore.TRINO_ENVIRONMENT; import static jakarta.ws.rs.core.MediaType.APPLICATION_OCTET_STREAM; -import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; -import static jakarta.ws.rs.core.Response.Status.FORBIDDEN; -import static jakarta.ws.rs.core.Response.Status.GONE; -import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static java.lang.Long.parseUnsignedLong; import static java.util.Objects.requireNonNull; @@ -73,10 +72,10 @@ public synchronized Response headRequest( { checkEnvironment(environment); if (!shards.containsKey(uuid)) { - return Response.status(NOT_FOUND).build(); + throw new NotFoundException(); } if (shards.get(uuid) == null) { - return Response.status(GONE).build(); + throw new GoneException(); } return Response.noContent().build(); } @@ -90,11 +89,11 @@ public synchronized Response getRequest( { checkEnvironment(environment); if (!shards.containsKey(uuid)) { - return Response.status(NOT_FOUND).build(); + throw new NotFoundException(); } byte[] bytes = shards.get(uuid); if (bytes == null) { - return Response.status(GONE).build(); + throw new GoneException(); } return Response.ok(bytes).build(); } @@ -110,15 +109,15 @@ public synchronized Response putRequest( { checkEnvironment(environment); if ((request.getContentLength() < 0) || (bytes.length != request.getContentLength())) { - return Response.status(BAD_REQUEST).build(); + throw new BadRequestException(); } if (parseUnsignedLong(hexHash, 16) != XxHash64.hash(Slices.wrappedBuffer(bytes))) { - return Response.status(BAD_REQUEST).build(); + throw new BadRequestException(); } if (shards.containsKey(uuid)) { byte[] existing = shards.get(uuid); if ((existing == null) || !Arrays.equals(bytes, existing)) { - return Response.status(FORBIDDEN).build(); + throw new ForbiddenException(); } } shards.put(uuid, bytes); @@ -133,10 +132,10 @@ public synchronized Response deleteRequest( { checkEnvironment(environment); if (!shards.containsKey(uuid)) { - return Response.status(NOT_FOUND).build(); + throw new NotFoundException(); } if (shards.get(uuid) == null) { - return Response.status(GONE).build(); + throw new GoneException(); } shards.put(uuid, null); return Response.noContent().build(); @@ -145,7 +144,7 @@ public synchronized Response deleteRequest( private void checkEnvironment(String environment) { if (!this.environment.equals(environment)) { - throw new WebApplicationException(Response.status(FORBIDDEN).build()); + throw new ForbiddenException(); } } } diff --git a/pom.xml b/pom.xml index 71f3386f3f7f..48f41df8b8af 100644 --- a/pom.xml +++ b/pom.xml @@ -182,7 +182,7 @@ 2.7.7-1 3.0.0 - 256 + 258 2.9.6 4.13.1 1.11.3