[pinpoint-apm#11145] Use Spring ProblemDetail to describe exceptions

ga-ram · Jun 12, 2024 · 8e9aaeb · 8e9aaeb
1 parent 4b880d4
commit 8e9aaeb
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 16 deletions.
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/controller/NonWhiteLabelErrorController.java b/web/src/main/java/com/navercorp/pinpoint/web/controller/NonWhiteLabelErrorController.java
@@ -24,12 +24,13 @@
 import org.springframework.boot.web.error.ErrorAttributeOptions;
 import org.springframework.boot.web.servlet.error.ErrorAttributes;
 import org.springframework.http.HttpStatus;
-import org.springframework.http.ResponseEntity;
+import org.springframework.http.ProblemDetail;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 
-import java.util.Map;
+import java.net.URI;
+import java.net.URISyntaxException;
 
 @RestController
 @RequestMapping(value={"/api/error"})
@@ -46,14 +47,20 @@ public NonWhiteLabelErrorController(
     }
 
     @RequestMapping
-    public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) {
+    public ProblemDetail error(HttpServletRequest request) {
         final HttpStatus status = this.getStatus(request);
-        if (status == HttpStatus.NO_CONTENT) {
-            return new ResponseEntity<>(status);
-        } else {
-            final Map<String, Object> body = this.getErrorAttributes(request, this.getErrorAttributeOptions(request));
-            return new ResponseEntity<>(body, status);
+        ProblemDetail ret = ProblemDetail.forStatus(status.value());
+        Object uri = request.getAttribute("jakarta.servlet.error.request_uri");
+        if (uri != null) {
+            try {
+                ret.setInstance(new URI(uri.toString()));
+            } catch (URISyntaxException e) {
+                ret.setInstance(null);
+            }
         }
+
+        ret.setProperties(this.getErrorAttributes(request, this.getErrorAttributeOptions(request)));
+        return ret;
     }
 
     private ErrorAttributeOptions getErrorAttributeOptions(HttpServletRequest request) {

diff --git a/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorAttributes.java b/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorAttributes.java
@@ -19,10 +19,18 @@ public PinpointErrorAttributes() {
     @Override
     public Map<String, Object> getErrorAttributes(WebRequest webRequest, ErrorAttributeOptions options) {
         Map<String, Object> errorAttributes = super.getErrorAttributes(webRequest, options);
+        this.removeDuplicateData(errorAttributes);
         this.addCustomData(webRequest, errorAttributes);
         return errorAttributes;
     }
 
+    // removes attributes already present with ProblemDetail
+    private void removeDuplicateData(Map<String, Object> errorAttributes) {
+        errorAttributes.remove("status");
+        errorAttributes.remove("error");        // ProblemDetail already has "title" field
+        errorAttributes.remove("path");        // ProblemDetail already has "instance" field
+    }
+
     private void addCustomData(WebRequest webRequest, Map<String, Object> errorAttributes) {
         PinpointErrorData pinpointErrorData = new PinpointErrorData(this.hostname, webRequest);
         errorAttributes.put("data", pinpointErrorData);

diff --git a/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorData.java b/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorData.java
@@ -31,19 +31,16 @@ public RequestInfo getRequestInfo() {
     public static class RequestInfo {
         private static final String UNKNOWN = "UNKNOWN";
         private final String method;
-        private final String url;
         private final Map<String, List<String>> headers;
         private final Map<String, String[]> parameters;
 
         public RequestInfo(WebRequest request) {
             if (request instanceof ServletWebRequest webRequest) {
                 this.method = webRequest.getRequest().getMethod();
-                this.url = String.valueOf(webRequest.getAttribute("jakarta.servlet.error.request_uri", 0));
                 this.headers = getRequestHeader(webRequest);
                 this.parameters = request.getParameterMap();
             } else {
                 this.method = "UNKNOWN";
-                this.url = "UNKNOWN";
                 this.headers = null;
                 this.parameters = null;
             }
@@ -53,10 +50,6 @@ public String getMethod() {
             return method;
         }
 
-        public String getUrl() {
-            return url;
-        }
-
         public Map<String, List<String>> getHeaders() {
             return headers;
         }
@@ -88,7 +81,6 @@ private Map<String, List<String>> getRequestHeader(ServletWebRequest webRequest)
         public String toString() {
             return "RequestInfo{" +
                     "method='" + method + '\'' +
-                    ", url='" + url + '\'' +
                     ", headers=" + headers +
                     ", parameters=" + parameters +
                     '}';