You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/02/03 02:51:16 UTC

[james-project] 10/12: JAMES-3495 Better factorize error handling between HTTP and WebSocket transport

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 <bt...@linagora.com>
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: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org