Implement Jetty request event handler

server/src/main/java/com/defold/extender/RequestErrorAdvice.java

@@ -0,0 +1,84 @@
+package com.defold.extender;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.RestControllerAdvice;
+import org.springframework.web.multipart.MaxUploadSizeExceededException;
+import org.springframework.web.multipart.MultipartException;
+
+import com.defold.extender.log.Markers;
+
+/**
+ * Handles uploads that fail while Jetty parses the multipart body. Parsing happens in
+ * DispatcherServlet.checkMultipart(), i.e. before the request is mapped to a controller method, so
+ * the @ExceptionHandler methods inside {@link ExtenderController} are never consulted for these
+ * failures - the request would otherwise end up in Spring's default /error handling, which
+ * answers with a bare "400 Bad Request" and logs nothing (see defold/extender#590).
+ */
+@RestControllerAdvice
+public class RequestErrorAdvice {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(RequestErrorAdvice.class);
+
+    // Jetty caps the number of multipart parts at ServletContextHandler.getMaxFormKeys()
+    // (server.jetty.max-form-keys) and reports it as: Form with too many keys [1043 > 1000]
+    private static final Pattern TOO_MANY_PARTS_RE = Pattern.compile("too many keys \\[(\\d+) > (\\d+)]");
+
+    private final String maxRequestSize;
+
+    public RequestErrorAdvice(@Value("${spring.servlet.multipart.max-request-size}") String maxRequestSize) {
+        this.maxRequestSize = maxRequestSize;
+    }
+
+    @ExceptionHandler(MultipartException.class)
+    public ResponseEntity<String> handleMultipartException(MultipartException ex) {
+        final Matcher tooManyParts = findTooManyParts(ex);
+        if (tooManyParts != null) {
+            final String message = String.format(
+                    "The build request contains too many files (%s, the limit is %s). Reduce the number of files in the project extensions, or update the editor to a version that uploads the sources as a single archive.",
+                    tooManyParts.group(1), tooManyParts.group(2));
+            LOGGER.error(Markers.SERVER_ERROR, message, ex);
+            return textResponse(HttpStatus.PAYLOAD_TOO_LARGE, message);
+        }
+
+        if (ex instanceof MaxUploadSizeExceededException) {
+            final String message = String.format("The build request is too large. Max allowed size is %s.", maxRequestSize);
+            LOGGER.error(Markers.SERVER_ERROR, message, ex);
+            return textResponse(HttpStatus.PAYLOAD_TOO_LARGE, message);
+        }
+
+        final String message = "The build request is not a valid multipart request and could not be read by the server.";
+        LOGGER.error(Markers.SERVER_ERROR, message, ex);
+        return textResponse(HttpStatus.BAD_REQUEST, message);
+    }
+
+    private static Matcher findTooManyParts(Throwable ex) {
+        for (Throwable cause = ex; cause != null; cause = cause.getCause()) {
+            if (cause.getMessage() != null) {
+                final Matcher matcher = TOO_MANY_PARTS_RE.matcher(cause.getMessage());
+                if (matcher.find()) {
+                    return matcher;
+                }
+            }
+            if (cause.getCause() == cause) {
+                break;
+            }
+        }
+        return null;
+    }
+
+    private static ResponseEntity<String> textResponse(HttpStatus status, String message) {
+        HttpHeaders headers = new HttpHeaders();
+        headers.setContentType(MediaType.TEXT_PLAIN);
+        return new ResponseEntity<>(message, headers, status);
+    }
+}

server/src/main/java/com/defold/extender/jetty/ExtenderJettyErrorHandler.java

@@ -0,0 +1,81 @@
+package com.defold.extender.jetty;
+
+import java.io.IOException;
+
+import org.eclipse.jetty.http.HttpException;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.io.Content;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.handler.ErrorHandler;
+import org.eclipse.jetty.util.Callback;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.defold.extender.log.Markers;
+
+/**
+ * Error handler installed on the Jetty {@link org.eclipse.jetty.server.Server}. Jetty uses it for
+ * requests that fail before they enter the servlet context - malformed HTTP, too large headers,
+ * too long URI, unsupported HTTP version. Those never reach Spring, so neither the controller
+ * exception handlers nor {@link com.defold.extender.RequestErrorAdvice} can see them; without this
+ * handler they are answered with an HTML page and are not logged at all.
+ *
+ * Errors raised inside the servlet context still go through Spring Boot's context error handler.
+ */
+public class ExtenderJettyErrorHandler extends ErrorHandler {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExtenderJettyErrorHandler.class);
+
+    private static final String MIME_TYPE = "text/plain";
+
+    public ExtenderJettyErrorHandler() {
+        setShowStacks(false);
+        setShowCauses(false);
+        setShowMessageInTitle(false);
+        setDefaultResponseMimeType(MIME_TYPE);
+    }
+
+    @Override
+    public boolean handle(Request request, Response response, Callback callback) throws Exception {
+        final String message = (String)request.getAttribute(ERROR_MESSAGE);
+        final Throwable cause = (Throwable)request.getAttribute(ERROR_EXCEPTION);
+        final int code = (cause instanceof HttpException httpException) ? httpException.getCode() : response.getStatus();
+
+        LOGGER.error(Markers.SERVER_ERROR, "Jetty rejected request {} {} from {} with status {}: {}",
+                request.getMethod(), request.getHttpURI(), Request.getRemoteAddr(request), code, message, cause);
+
+        return super.handle(request, response, callback);
+    }
+
+    @Override
+    protected void generateResponse(Request request, Response response, int code, String message, Throwable cause, Callback callback) throws IOException {
+        response.getHeaders().put(HttpHeader.CONTENT_TYPE, MIME_TYPE + ";charset=utf-8");
+        Content.Sink.write(response, true, describe(code), callback);
+    }
+
+    /**
+     * The body the user gets to see. Only the status is derived from the failure, the details of
+     * what Jetty disliked stay in the log.
+     */
+    static String describe(int code) {
+        final String explanation = switch (code) {
+            case HttpStatus.BAD_REQUEST_400 ->
+                "The server could not parse the request, so the build was never started. This usually means the request was truncated or malformed on the way to the server.";
+            case HttpStatus.REQUEST_TIMEOUT_408 ->
+                "The server timed out while waiting for the request to be sent.";
+            case HttpStatus.PAYLOAD_TOO_LARGE_413 ->
+                "The build request is too large.";
+            case HttpStatus.URI_TOO_LONG_414 ->
+                "The request URI is too long.";
+            case HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431 ->
+                "The request headers are too large.";
+            case HttpStatus.HTTP_VERSION_NOT_SUPPORTED_505 ->
+                "The HTTP version used by the client is not supported by the server.";
+            default ->
+                "The request was rejected by the server before the build was started.";
+        };
+        return String.format("%d %s%n%n%s%n", code, HttpStatus.getMessage(code), explanation);
+    }
+}

server/src/main/java/com/defold/extender/jetty/JettyRequestEventsHandler.java

@@ -0,0 +1,38 @@
+package com.defold.extender.jetty;
+
+import org.eclipse.jetty.http.HttpFields;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.handler.EventsHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.defold.extender.log.Markers;
+
+/**
+ * Logs requests that fail on the Jetty side. This complements {@link ExtenderJettyErrorHandler}:
+ * the error handler only runs when Jetty generates an error response, while this handler also sees
+ * failures that never produce one - a client that aborts mid upload, an idle timeout, or a failure
+ * after the response was already committed.
+ */
+public class JettyRequestEventsHandler extends EventsHandler {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(JettyRequestEventsHandler.class);
+
+    // Compilation errors are reported as 422 by ExtenderController.handleExtenderException, which
+    // logs them with the full build output. No need to log them a second time here.
+    private static final int COMPILATION_ERROR_STATUS = 422;
+
+    @Override
+    protected void onComplete(Request request, int status, HttpFields headers, Throwable failure) {
+        if (failure != null) {
+            LOGGER.error(Markers.SERVER_ERROR, "Request {} {} from {} failed with status {}",
+                    request.getMethod(), request.getHttpURI(), Request.getRemoteAddr(request), status, failure);
+        } else if (status >= 500) {
+            LOGGER.error(Markers.SERVER_ERROR, "Request {} {} from {} completed with status {}",
+                    request.getMethod(), request.getHttpURI(), Request.getRemoteAddr(request), status);
+        } else if (status >= 400 && status != COMPILATION_ERROR_STATUS) {
+            LOGGER.warn("Request {} {} from {} completed with status {}",
+                    request.getMethod(), request.getHttpURI(), Request.getRemoteAddr(request), status);
+        }
+    }
+}

server/src/main/java/com/defold/extender/jetty/JettyServerConfiguration.java

@@ -0,0 +1,25 @@
+package com.defold.extender.jetty;
+
+import org.eclipse.jetty.server.Handler;
+import org.springframework.boot.jetty.ConfigurableJettyWebServerFactory;
+import org.springframework.boot.web.server.WebServerFactoryCustomizer;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration(proxyBeanMethods = false)
+public class JettyServerConfiguration {
+
+    @Bean
+    WebServerFactoryCustomizer<ConfigurableJettyWebServerFactory> extenderJettyCustomizer() {
+        return factory -> factory.addServerCustomizers(server -> {
+            // Spring Boot sets its own error handler on the servlet context (it drives the /error
+            // dispatch), the server level one is unused and is where pre-context errors end up.
+            server.setErrorHandler(new ExtenderJettyErrorHandler());
+
+            final JettyRequestEventsHandler eventsHandler = new JettyRequestEventsHandler();
+            final Handler currentHandler = server.getHandler();
+            eventsHandler.setHandler(currentHandler);
+            server.setHandler(eventsHandler);
+        });
+    }
+}

server/src/main/resources/application.yml

@@ -1,7 +1,12 @@
 server:
     port: 9000
+    # Requests with larger headers are rejected by Jetty before any application code runs
+    max-http-request-header-size: 16KB
     jetty:
         connection-idle-timeout: 600000
+        # Jetty uses maxFormKeys as the maximum number of multipart parts. The client uploads one
+        # part per file, so this is effectively the max number of files in a build request.
+        max-form-keys: 5000
 
 extender:
     sdk: