This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 8401dd43c84823cfc79c61ac07b7f05d93221e5d Author: Benoit Tellier <[email protected]> AuthorDate: Fri Jan 29 10:33:56 2021 +0700 JAMES-3495 Better factorize error handling between HTTP and WebSocket transport --- .../apache/james/jmap/core/ProblemDetails.scala | 25 ++++++++++- .../apache/james/jmap/routes/JMAPApiRoutes.scala | 35 +++------------- .../apache/james/jmap/routes/WebSocketRoutes.scala | 48 +++++----------------- 3 files changed, 41 insertions(+), 67 deletions(-) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/ProblemDetails.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/ProblemDetails.scala index 9547f00..85c223d 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/ProblemDetails.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/ProblemDetails.scala @@ -18,9 +18,13 @@ ****************************************************************/ package org.apache.james.jmap.core +import com.fasterxml.jackson.core.JsonParseException import io.netty.handler.codec.http.HttpResponseStatus -import io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST +import io.netty.handler.codec.http.HttpResponseStatus.{BAD_REQUEST, INTERNAL_SERVER_ERROR, UNAUTHORIZED} import org.apache.james.jmap.core.RequestLevelErrorType.{DEFAULT_ERROR_TYPE, ErrorTypeIdentifier} +import org.apache.james.jmap.exceptions.UnauthorizedException +import org.apache.james.jmap.routes.UnsupportedCapabilitiesException +import org.slf4j.{Logger, LoggerFactory} /** * Problem Details for HTTP APIs within the JMAP context @@ -33,6 +37,25 @@ case class ProblemDetails(`type`: ErrorTypeIdentifier = DEFAULT_ERROR_TYPE, detail: String) object ProblemDetails { + val LOGGER: Logger = LoggerFactory.getLogger(classOf[ProblemDetails]) + + def forThrowable(throwable: Throwable): ProblemDetails = throwable match { + case exception: IllegalArgumentException => + notRequestProblem( + s"The request was successfully parsed as JSON but did not match the type signature of the Request object: ${exception.getMessage}") + case e: UnauthorizedException => + LOGGER.warn("Unauthorized", e) + ProblemDetails(status = UNAUTHORIZED, detail = e.getMessage) + case exception: JsonParseException => + notJSONProblem( + s"The content type of the request was not application/json or the request did not parse as I-JSON: ${exception.getMessage}") + case exception: UnsupportedCapabilitiesException => + unknownCapabilityProblem(s"The request used unsupported capabilities: ${exception.capabilities}") + case e => + LOGGER.error("Unexpected error upon API request", e) + ProblemDetails(status = INTERNAL_SERVER_ERROR, detail = e.getMessage) + } + def notRequestProblem(message: String): ProblemDetails = ProblemDetails(RequestLevelErrorType.NOT_REQUEST, BAD_REQUEST, None, message) def notJSONProblem(message: String): ProblemDetails = ProblemDetails(RequestLevelErrorType.NOT_JSON, BAD_REQUEST, None, message) def unknownCapabilityProblem(message: String): ProblemDetails = ProblemDetails(RequestLevelErrorType.UNKNOWN_CAPABILITY, BAD_REQUEST, None, message) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala index 2bc48bb..a09b4cc 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala @@ -23,21 +23,17 @@ import java.nio.charset.StandardCharsets import java.util.stream import java.util.stream.Stream -import com.fasterxml.jackson.core.JsonParseException import io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE -import io.netty.handler.codec.http.HttpResponseStatus.{BAD_REQUEST, INTERNAL_SERVER_ERROR, OK, UNAUTHORIZED} -import io.netty.handler.codec.http.{HttpMethod, HttpResponseStatus} +import io.netty.handler.codec.http.HttpMethod +import io.netty.handler.codec.http.HttpResponseStatus.OK import javax.inject.{Inject, Named} import org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE import org.apache.james.jmap.JMAPUrls.JMAP import org.apache.james.jmap.core.CapabilityIdentifier.CapabilityIdentifier -import org.apache.james.jmap.core.ProblemDetails.{notJSONProblem, notRequestProblem, unknownCapabilityProblem} import org.apache.james.jmap.core.{ProblemDetails, RequestObject} -import org.apache.james.jmap.exceptions.UnauthorizedException import org.apache.james.jmap.http.rfc8621.InjectionKeys import org.apache.james.jmap.http.{Authenticator, UserProvisioning} import org.apache.james.jmap.json.ResponseSerializer -import org.apache.james.jmap.routes.DownloadRoutes.LOGGER import org.apache.james.jmap.{Endpoint, JMAPRoute, JMAPRoutes} import org.apache.james.mailbox.MailboxSession import org.slf4j.{Logger, LoggerFactory} @@ -104,30 +100,11 @@ class JMAPApiRoutes @Inject() (@Named(InjectionKeys.RFC_8621) val authenticator: StandardCharsets.UTF_8) .`then`())) - private def handleError(throwable: Throwable, response: HttpServerResponse): SMono[Void] = throwable match { - case exception: IllegalArgumentException => respondDetails(response, - notRequestProblem( - s"The request was successfully parsed as JSON but did not match the type signature of the Request object: ${exception.getMessage}")) + private def handleError(throwable: Throwable, response: HttpServerResponse): SMono[Void] = + respondDetails(response, ProblemDetails.forThrowable(throwable)) - case e: UnauthorizedException => - LOGGER.warn("Unauthorized", e) - respondDetails(response, - ProblemDetails(status = UNAUTHORIZED, detail = e.getMessage), - UNAUTHORIZED) - case exception: JsonParseException => respondDetails(response, - notJSONProblem( - s"The content type of the request was not application/json or the request did not parse as I-JSON: ${exception.getMessage}")) - case exception: UnsupportedCapabilitiesException => respondDetails(response, - unknownCapabilityProblem(s"The request used unsupported capabilities: ${exception.capabilities}")) - case e => - LOGGER.error("Unexpected error upon API request", e) - respondDetails(response, - ProblemDetails(status = INTERNAL_SERVER_ERROR, detail = e.getMessage), - INTERNAL_SERVER_ERROR) - } - - private def respondDetails(httpServerResponse: HttpServerResponse, details: ProblemDetails, statusCode: HttpResponseStatus = BAD_REQUEST): SMono[Void] = - SMono.fromPublisher(httpServerResponse.status(statusCode) + private def respondDetails(httpServerResponse: HttpServerResponse, details: ProblemDetails): SMono[Void] = + SMono.fromPublisher(httpServerResponse.status(details.status) .header(CONTENT_TYPE, JSON_CONTENT_TYPE) .sendString(SMono.fromCallable(() => ResponseSerializer.serialize(details).toString), StandardCharsets.UTF_8) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala index c4b2900..3965ac7 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala @@ -19,20 +19,19 @@ package org.apache.james.jmap.routes -import com.fasterxml.jackson.core.JsonParseException +import java.nio.charset.StandardCharsets +import java.util.stream + import io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE -import io.netty.handler.codec.http.HttpResponseStatus.{BAD_REQUEST, INTERNAL_SERVER_ERROR, UNAUTHORIZED} +import io.netty.handler.codec.http.HttpMethod import io.netty.handler.codec.http.websocketx.WebSocketFrame -import io.netty.handler.codec.http.{HttpMethod, HttpResponseStatus} +import javax.inject.{Inject, Named} import org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE import org.apache.james.jmap.JMAPUrls.JMAP_WS -import org.apache.james.jmap.core.ProblemDetails.{notJSONProblem, notRequestProblem, unknownCapabilityProblem} import org.apache.james.jmap.core.{ProblemDetails, RequestId, WebSocketError, WebSocketOutboundMessage, WebSocketRequest, WebSocketResponse} -import org.apache.james.jmap.exceptions.UnauthorizedException import org.apache.james.jmap.http.rfc8621.InjectionKeys import org.apache.james.jmap.http.{Authenticator, UserProvisioning} import org.apache.james.jmap.json.ResponseSerializer -import org.apache.james.jmap.routes.WebSocketRoutes.LOGGER import org.apache.james.jmap.{Endpoint, JMAPRoute, JMAPRoutes} import org.apache.james.mailbox.MailboxSession import org.slf4j.{Logger, LoggerFactory} @@ -42,10 +41,6 @@ import reactor.core.scheduler.Schedulers import reactor.netty.http.server.{HttpServerRequest, HttpServerResponse} import reactor.netty.http.websocket.{WebsocketInbound, WebsocketOutbound} -import java.nio.charset.StandardCharsets -import java.util.stream -import javax.inject.{Inject, Named} - object WebSocketRoutes { val LOGGER: Logger = LoggerFactory.getLogger(classOf[WebSocketRoutes]) } @@ -109,35 +104,14 @@ class WebSocketRoutes @Inject() (@Named(InjectionKeys.RFC_8621) val authenticato .subscribeOn(Schedulers.elastic) }) - private def handleHttpHandshakeError(throwable: Throwable, response: HttpServerResponse): SMono[Void] = throwable match { - case e: UnauthorizedException => - LOGGER.warn("Unauthorized", e) - respondDetails(response, - ProblemDetails(status = UNAUTHORIZED, detail = e.getMessage), - UNAUTHORIZED) - case e => - LOGGER.error("Unexpected error upon WebSocket handshake request", e) - respondDetails(response, - ProblemDetails(status = INTERNAL_SERVER_ERROR, detail = e.getMessage), - INTERNAL_SERVER_ERROR) - } + private def handleHttpHandshakeError(throwable: Throwable, response: HttpServerResponse): SMono[Void] = + respondDetails(response, ProblemDetails.forThrowable(throwable)) - private def asError(requestId: Option[RequestId])(throwable: Throwable): WebSocketError = throwable match { - case exception: IllegalArgumentException => - WebSocketError(requestId, notRequestProblem( - s"The request was successfully parsed as JSON but did not match the type signature of the Request object: ${exception.getMessage}")) - case exception: JsonParseException => - WebSocketError(requestId, notJSONProblem( - s"The content type of the request was not application/json or the request did not parse as I-JSON: ${exception.getMessage}")) - case exception: UnsupportedCapabilitiesException => - WebSocketError(requestId, unknownCapabilityProblem(s"The request used unsupported capabilities: ${exception.capabilities}")) - case e => - LOGGER.error("Unexpected error upon API request", e) - WebSocketError(requestId, ProblemDetails(status = INTERNAL_SERVER_ERROR, detail = e.getMessage)) - } + private def asError(requestId: Option[RequestId])(throwable: Throwable): WebSocketError = + WebSocketError(requestId, ProblemDetails.forThrowable(throwable)) - private def respondDetails(httpServerResponse: HttpServerResponse, details: ProblemDetails, statusCode: HttpResponseStatus = BAD_REQUEST): SMono[Void] = - SMono.fromPublisher(httpServerResponse.status(statusCode) + private def respondDetails(httpServerResponse: HttpServerResponse, details: ProblemDetails): SMono[Void] = + SMono.fromPublisher(httpServerResponse.status(details.status) .header(CONTENT_TYPE, JSON_CONTENT_TYPE) .sendString(SMono.fromCallable(() => ResponseSerializer.serialize(details).toString), StandardCharsets.UTF_8) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
