Fix 3443 to set appropriate httpstatus codes and messages for different types of BlockException

sentinel-adapter/sentinel-spring-webmvc-adapter/pom.xml

@@ -23,6 +23,10 @@
             <groupId>com.alibaba.csp</groupId>
             <artifactId>sentinel-core</artifactId>
         </dependency>
+        <dependency>
+            <groupId>com.alibaba.csp</groupId>
+            <artifactId>sentinel-web-adapter-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>javax.servlet</groupId>
             <artifactId>javax.servlet-api</artifactId>

sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/callback/DefaultBlockExceptionHandler.java

@@ -15,8 +15,8 @@
  */
 package com.alibaba.csp.sentinel.adapter.spring.webmvc.callback;
 
+import com.alibaba.csp.sentinel.adapter.web.common.DefaultBlockExceptionResponse;
 import com.alibaba.csp.sentinel.slots.block.BlockException;
-import com.alibaba.csp.sentinel.util.StringUtil;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -31,11 +31,10 @@ public class DefaultBlockExceptionHandler implements BlockExceptionHandler {
 
     @Override
     public void handle(HttpServletRequest request, HttpServletResponse response, BlockException e) throws Exception {
-        // Return 429 (Too Many Requests) by default.
-        response.setStatus(429);
-
+        DefaultBlockExceptionResponse expRes = DefaultBlockExceptionResponse.resolve(e.getClass());
+        response.setStatus(expRes.getStatus());
         PrintWriter out = response.getWriter();
-        out.print("Blocked by Sentinel (flow limiting)");
+        out.print(expRes.getMsg());
         out.flush();
         out.close();
     }

sentinel-adapter/sentinel-spring-webmvc-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/SentinelDefaultBlockExceptionHandlerTest.java

@@ -0,0 +1,158 @@
+package com.alibaba.csp.sentinel.adapter.spring.webmvc;
+
+import com.alibaba.csp.sentinel.adapter.spring.webmvc.config.DefaultInterceptorConfig;
+import com.alibaba.csp.sentinel.adapter.spring.webmvc.config.InterceptorConfig;
+import com.alibaba.csp.sentinel.adapter.web.common.DefaultBlockExceptionResponse;
+import com.alibaba.csp.sentinel.node.ClusterNode;
+import com.alibaba.csp.sentinel.slots.block.RuleConstant;
+import com.alibaba.csp.sentinel.slots.block.degrade.DegradeException;
+import com.alibaba.csp.sentinel.slots.block.degrade.DegradeRule;
+import com.alibaba.csp.sentinel.slots.block.degrade.DegradeRuleManager;
+import com.alibaba.csp.sentinel.slots.block.flow.FlowException;
+import com.alibaba.csp.sentinel.slots.block.flow.FlowRule;
+import com.alibaba.csp.sentinel.slots.block.flow.FlowRuleManager;
+import com.alibaba.csp.sentinel.slots.clusterbuilder.ClusterBuilderSlot;
+import com.alibaba.csp.sentinel.util.StringUtil;
+import org.junit.After;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
+import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.FilterType;
+import org.springframework.context.annotation.Import;
+import org.springframework.http.MediaType;
+import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.test.web.servlet.MockMvc;
+
+import java.util.Collections;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+/**
+ * @author Lingzhi
+ */
+@RunWith(SpringRunner.class)
+@Import(DefaultInterceptorConfig.class)
+@WebMvcTest(excludeFilters = @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, classes = InterceptorConfig.class))
+public class SentinelDefaultBlockExceptionHandlerTest {
+    @Autowired
+    private MockMvc mvc;
+
+    @Test
+    public void testOriginParser() throws Exception {
+        String springMvcPathVariableUrl = "/foo/{id}";
+        String limitOrigin = "userA";
+        final String headerName = "S-User";
+        configureRulesFor(springMvcPathVariableUrl, 0, limitOrigin);
+
+        // This will be passed since the caller is different: userB
+        this.mvc.perform(get("/foo/1").accept(MediaType.TEXT_PLAIN).header(headerName, "userB"))
+                .andExpect(status().isOk())
+                .andExpect(content().string("foo 1"));
+
+        // This will be blocked since the caller is same: userA
+        DefaultBlockExceptionResponse res = DefaultBlockExceptionResponse.FLOW_EXCEPTION;
+        this.mvc.perform(
+                        get("/foo/2").accept(MediaType.APPLICATION_JSON).header(headerName, limitOrigin))
+                .andExpect(status().is(res.getStatus()))
+                .andExpect(content().string(res.getMsg()));
+
+        // This will be passed since the caller is different: ""
+        this.mvc.perform(get("/foo/3").accept(MediaType.APPLICATION_JSON))
+                .andExpect(status().isOk())
+                .andExpect(content().string("foo 3"));
+    }
+
+    @Test
+    public void testRuntimeException() throws Exception {
+        String url = "/runtimeException";
+        configureExceptionRulesFor(url, 3, null);
+        int repeat = 3;
+        for (int i = 0; i < repeat; i++) {
+            this.mvc.perform(get(url))
+                    .andExpect(status().isOk())
+                    .andExpect(content().string(ResultWrapper.error().toJsonString()));
+            ClusterNode cn = ClusterBuilderSlot.getClusterNode(url);
+            assertNotNull(cn);
+            assertEquals(i + 1, cn.passQps(), 0.01);
+        }
+
+        // This will be blocked and response json.
+        DefaultBlockExceptionResponse res = DefaultBlockExceptionResponse.resolve(FlowException.class);
+        this.mvc.perform(get(url))
+                .andExpect(status().is(res.getStatus()))
+                .andExpect(content().string(res.getMsg()));
+        ClusterNode cn = ClusterBuilderSlot.getClusterNode(url);
+        assertNotNull(cn);
+        assertEquals(repeat, cn.passQps(), 0.01);
+        assertEquals(1, cn.blockRequest(), 1);
+    }
+
+
+    @Test
+    public void testExceptionPerception() throws Exception {
+        String url = "/bizException";
+        configureExceptionDegradeRulesFor(url, 2.6, null);
+        int repeat = 3;
+        for (int i = 0; i < repeat; i++) {
+            this.mvc.perform(get(url))
+                    .andExpect(status().isOk())
+                    .andExpect(content().string(new ResultWrapper(-1, "Biz error").toJsonString()));
+
+            ClusterNode cn = ClusterBuilderSlot.getClusterNode(url);
+            assertNotNull(cn);
+            assertEquals(i + 1, cn.passQps(), 0.01);
+        }
+
+        // This will be blocked and response.
+        DefaultBlockExceptionResponse res = DefaultBlockExceptionResponse.resolve(DegradeException.class);
+        this.mvc.perform(get(url))
+                .andExpect(status().is(res.getStatus()))
+                .andExpect(content().string(res.getMsg()));
+        ClusterNode cn = ClusterBuilderSlot.getClusterNode(url);
+        assertNotNull(cn);
+        assertEquals(repeat, cn.passQps(), 0.01);
+        assertEquals(1, cn.blockRequest(), 1);
+    }
+
+    private void configureRulesFor(String resource, int count, String limitApp) {
+        FlowRule rule = new FlowRule().setCount(count).setGrade(RuleConstant.FLOW_GRADE_QPS);
+        rule.setResource(resource);
+        if (StringUtil.isNotBlank(limitApp)) {
+            rule.setLimitApp(limitApp);
+        }
+        FlowRuleManager.loadRules(Collections.singletonList(rule));
+    }
+
+    private void configureExceptionRulesFor(String resource, int count, String limitApp) {
+        FlowRule rule = new FlowRule().setCount(count).setGrade(RuleConstant.DEGRADE_GRADE_EXCEPTION_RATIO);
+        rule.setResource(resource);
+        if (StringUtil.isNotBlank(limitApp)) {
+            rule.setLimitApp(limitApp);
+        }
+        FlowRuleManager.loadRules(Collections.singletonList(rule));
+    }
+
+    private void configureExceptionDegradeRulesFor(String resource, double count, String limitApp) {
+        DegradeRule rule = new DegradeRule().setCount(count)
+                .setStatIntervalMs(1000).setMinRequestAmount(1)
+                .setTimeWindow(5).setGrade(RuleConstant.DEGRADE_GRADE_EXCEPTION_COUNT);
+        rule.setResource(resource);
+        if (StringUtil.isNotBlank(limitApp)) {
+            rule.setLimitApp(limitApp);
+        }
+        DegradeRuleManager.loadRules(Collections.singletonList(rule));
+    }
+
+    @After
+    public void cleanUp() {
+        FlowRuleManager.loadRules(null);
+        DegradeRuleManager.loadRules(null);
+        ClusterBuilderSlot.resetClusterNodes();
+    }
+}

sentinel-adapter/sentinel-spring-webmvc-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/SentinelSpringMvcIntegrationTest.java

@@ -15,13 +15,6 @@
  */
 package com.alibaba.csp.sentinel.adapter.spring.webmvc;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
-
 import com.alibaba.csp.sentinel.context.ContextUtil;
 import com.alibaba.csp.sentinel.node.ClusterNode;
 import com.alibaba.csp.sentinel.slots.block.RuleConstant;
@@ -31,9 +24,6 @@
 import com.alibaba.csp.sentinel.slots.block.flow.FlowRuleManager;
 import com.alibaba.csp.sentinel.slots.clusterbuilder.ClusterBuilderSlot;
 import com.alibaba.csp.sentinel.util.StringUtil;
-
-import java.util.Collections;
-
 import org.junit.After;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -44,6 +34,13 @@
 import org.springframework.test.context.junit4.SpringRunner;
 import org.springframework.test.web.servlet.MockMvc;
 
+import java.util.Collections;
+
+import static org.junit.Assert.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
 /**
  * @author kaizi2009
  */
@@ -94,16 +91,14 @@ public void testOriginParser() throws Exception {
 
         // This will be blocked since the caller is same: userA
         this.mvc.perform(
-                get("/foo/2").accept(MediaType.APPLICATION_JSON).header(headerName, limitOrigin))
+                        get("/foo/2").accept(MediaType.APPLICATION_JSON).header(headerName, limitOrigin))
                 .andExpect(status().isOk())
                 .andExpect(content().json(ResultWrapper.blocked().toJsonString()));
 
         // This will be passed since the caller is different: ""
         this.mvc.perform(get("/foo/3").accept(MediaType.APPLICATION_JSON))
                 .andExpect(status().isOk())
                 .andExpect(content().string("foo 3"));
-
-        FlowRuleManager.loadRules(null);
     }
 
     @Test
@@ -209,6 +204,7 @@ private void configureExceptionDegradeRulesFor(String resource, double count, St
     @After
     public void cleanUp() {
         FlowRuleManager.loadRules(null);
+        DegradeRuleManager.loadRules(null);
         ClusterBuilderSlot.resetClusterNodes();
     }
 }

sentinel-adapter/sentinel-spring-webmvc-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/config/DefaultInterceptorConfig.java

@@ -0,0 +1,60 @@
+package com.alibaba.csp.sentinel.adapter.spring.webmvc.config;
+
+import com.alibaba.csp.sentinel.adapter.spring.webmvc.SentinelExceptionAware;
+import com.alibaba.csp.sentinel.adapter.spring.webmvc.SentinelWebInterceptor;
+import com.alibaba.csp.sentinel.adapter.spring.webmvc.SentinelWebTotalInterceptor;
+import com.alibaba.csp.sentinel.adapter.spring.webmvc.callback.DefaultBlockExceptionHandler;
+import org.springframework.boot.test.context.TestConfiguration;
+import org.springframework.context.annotation.Bean;
+import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
+import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
+
+/**
+ * Interceptor Config using DefaultBlockExceptionHandler
+ *
+ * @author Lingzhi
+ */
+@TestConfiguration
+public class DefaultInterceptorConfig implements WebMvcConfigurer {
+
+    @Bean
+    public SentinelExceptionAware sentinelExceptionAware() {
+        return new SentinelExceptionAware();
+    }
+
+    @Override
+    public void addInterceptors(InterceptorRegistry registry) {
+        //Add sentinel interceptor
+        addSpringMvcInterceptor(registry);
+
+        //If you want to sentinel the total flow, you can add total interceptor
+        addSpringMvcTotalInterceptor(registry);
+    }
+
+    private void addSpringMvcInterceptor(InterceptorRegistry registry) {
+        //Config
+        SentinelWebMvcConfig config = new SentinelWebMvcConfig();
+
+        config.setBlockExceptionHandler(new DefaultBlockExceptionHandler());
+
+        //Custom configuration if necessary
+        config.setHttpMethodSpecify(false);
+        config.setWebContextUnify(true);
+        config.setOriginParser(request -> request.getHeader("S-user"));
+
+        //Add sentinel interceptor
+        registry.addInterceptor(new SentinelWebInterceptor(config)).addPathPatterns("/**");
+    }
+
+    private void addSpringMvcTotalInterceptor(InterceptorRegistry registry) {
+        //Configure
+        SentinelWebMvcTotalConfig config = new SentinelWebMvcTotalConfig();
+
+        //Custom configuration if necessary
+        config.setRequestAttributeName("my_sentinel_spring_mvc_total_entity_container");
+        config.setTotalResourceName("my_spring_mvc_total_url_request");
+
+        //Add sentinel interceptor
+        registry.addInterceptor(new SentinelWebTotalInterceptor(config)).addPathPatterns("/**");
+    }
+}

sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/AbstractSentinelInterceptor.java

@@ -15,11 +15,7 @@
  */
 package com.alibaba.csp.sentinel.adapter.spring.webmvc_v6x;
 
-import com.alibaba.csp.sentinel.Entry;
-import com.alibaba.csp.sentinel.EntryType;
-import com.alibaba.csp.sentinel.ResourceTypeConstants;
-import com.alibaba.csp.sentinel.SphU;
-import com.alibaba.csp.sentinel.Tracer;
+import com.alibaba.csp.sentinel.*;
 import com.alibaba.csp.sentinel.adapter.spring.webmvc_v6x.config.BaseWebMvcConfig;
 import com.alibaba.csp.sentinel.context.ContextUtil;
 import com.alibaba.csp.sentinel.log.RecordLog;
@@ -28,10 +24,14 @@
 import com.alibaba.csp.sentinel.util.StringUtil;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import org.springframework.web.context.request.RequestContextHolder;
+import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.servlet.AsyncHandlerInterceptor;
 import org.springframework.web.servlet.ModelAndView;
 
+import java.util.Objects;
+
 /**
  * Since request may be reprocessed in flow if any forwarding or including or other action
  * happened (see {@link jakarta.servlet.ServletRequest#getDispatcherType()}) we will only
@@ -74,7 +74,7 @@ private Integer increaseReference(HttpServletRequest request, String rcKey, int
 
         if (obj == null) {
             // initial
-            obj = Integer.valueOf(0);
+            obj = 0;
         }
 
         Integer newRc = (Integer) obj + step;
@@ -193,12 +193,21 @@ protected void removeEntryInRequest(HttpServletRequest request) {
     }
 
     protected void traceExceptionAndExit(Entry entry, Exception ex) {
-        if (entry != null) {
-            if (ex != null) {
-                Tracer.traceEntry(ex, entry);
-            }
-            entry.exit();
+        if (entry == null) {
+            return;
+        }
+        HttpServletRequest request = getHttpServletRequest();
+        if (request != null
+                && ex == null
+                && increaseReference(request, this.baseWebMvcConfig.getRequestRefName() + ":" + BaseWebMvcConfig.REQUEST_REF_EXCEPTION_NAME, 1) == 1) {
+            //Each interceptor can only catch exception once
+            ex = (Exception) request.getAttribute(BaseWebMvcConfig.REQUEST_REF_EXCEPTION_NAME);
         }
+
+        if (ex != null) {
+            Tracer.traceEntry(ex, entry);
+        }
+        entry.exit();
     }
 
     protected void handleBlockException(HttpServletRequest request, HttpServletResponse response, String resourceName,
@@ -228,4 +237,10 @@ protected String parseOrigin(HttpServletRequest request) {
         return origin;
     }
 
+    private HttpServletRequest getHttpServletRequest() {
+        ServletRequestAttributes servletRequestAttributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
+
+        return Objects.isNull(servletRequestAttributes) ? null : servletRequestAttributes.getRequest();
+    }
+
 }