diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java index b2040dca42..b0c38e2e36 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java @@ -102,15 +102,7 @@ protected Object handleFallback(ProceedingJoinPoint pjp, String fallback, String args[args.length - 1] = ex; } - try { - if (isStatic(fallbackMethod)) { - return fallbackMethod.invoke(null, args); - } - return fallbackMethod.invoke(pjp.getTarget(), args); - } catch (InvocationTargetException e) { - // throw the actual exception - throw e.getTargetException(); - } + return invoke(pjp, fallbackMethod, args); } // If fallback is absent, we'll try the defaultFallback if provided. return handleDefaultFallback(pjp, defaultFallback, fallbackClass, ex); @@ -123,15 +115,7 @@ protected Object handleDefaultFallback(ProceedingJoinPoint pjp, String defaultFa if (fallbackMethod != null) { // Construct args. Object[] args = fallbackMethod.getParameterTypes().length == 0 ? new Object[0] : new Object[] {ex}; - try { - if (isStatic(fallbackMethod)) { - return fallbackMethod.invoke(null, args); - } - return fallbackMethod.invoke(pjp.getTarget(), args); - } catch (InvocationTargetException e) { - // throw the actual exception - throw e.getTargetException(); - } + return invoke(pjp, fallbackMethod, args); } // If no any fallback is present, then directly throw the exception. @@ -149,21 +133,44 @@ protected Object handleBlockException(ProceedingJoinPoint pjp, SentinelResource // Construct args. Object[] args = Arrays.copyOf(originArgs, originArgs.length + 1); args[args.length - 1] = ex; - try { - if (isStatic(blockHandlerMethod)) { - return blockHandlerMethod.invoke(null, args); - } - return blockHandlerMethod.invoke(pjp.getTarget(), args); - } catch (InvocationTargetException e) { - // throw the actual exception - throw e.getTargetException(); - } + return invoke(pjp, blockHandlerMethod, args); } // If no block handler is present, then go to fallback. return handleFallback(pjp, annotation, ex); } + private Object invoke(ProceedingJoinPoint pjp, Method method, Object[] args) throws Throwable { + try { + if (!method.isAccessible()) { + makeAccessible(method); + } + if (isStatic(method)) { + return method.invoke(null, args); + } + return method.invoke(pjp.getTarget(), args); + } catch (InvocationTargetException e) { + // throw the actual exception + throw e.getTargetException(); + } + } + + /** + * Make the given method accessible, explicitly setting it accessible if + * necessary. The {@code setAccessible(true)} method is only called + * when actually necessary, to avoid unnecessary conflicts with a JVM + * SecurityManager (if active). + * @param method the method to make accessible + * @see java.lang.reflect.Method#setAccessible + */ + private static void makeAccessible(Method method) { + boolean isNotPublic = !Modifier.isPublic(method.getModifiers()) || + !Modifier.isPublic(method.getDeclaringClass().getModifiers()); + if (isNotPublic && !method.isAccessible()) { + method.setAccessible(true); + } + } + private Method extractFallbackMethod(ProceedingJoinPoint pjp, String fallbackName, Class[] locationClass) { if (StringUtil.isBlank(fallbackName)) { return null; diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java index 370df67ac7..e5da0413f1 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java @@ -208,6 +208,28 @@ public void testClassLevelDefaultFallbackWithSingleParam() { assertThat(cn1.blockQps()).isZero(); } + @Test + public void testFallBackPrivateMethod() throws Exception { + String resourceName = "apiFooWithFallback"; + ClusterNode cn = ClusterBuilderSlot.getClusterNode(resourceName); + + try { + fooService.fooWithPrivateFallback(5758); + fail("should not reach here"); + } catch (Exception ex) { + // Should not be traced. + assertThat(cn.exceptionQps()).isZero(); + } + + assertThat(fooService.fooWithPrivateFallback(5763)).isEqualTo("EEE..."); + + // Test for blockHandler + FlowRuleManager.loadRules(Collections.singletonList( + new FlowRule(resourceName).setCount(0) + )); + assertThat(fooService.fooWithPrivateFallback(2221)).isEqualTo("Oops, 2221"); + } + @Before public void setUp() throws Exception { FlowRuleManager.loadRules(new ArrayList()); diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java index a8dfe1bc3f..3505c7e80d 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java @@ -76,6 +76,18 @@ public String baz(String name) { return "cheers, " + name; } + @SentinelResource(value = "apiFooWithFallback", blockHandler = "fooBlockHandlerPrivate", fallback = "fooFallbackFuncPrivate", + exceptionsToTrace = {IllegalArgumentException.class}) + public String fooWithPrivateFallback(int i) throws Exception { + if (i == 5758) { + throw new IllegalAccessException(); + } + if (i == 5763) { + throw new IllegalArgumentException(); + } + return "Hello for " + i; + } + public String fooBlockHandler(int i, BlockException ex) { return "Oops, " + i; } @@ -83,4 +95,12 @@ public String fooBlockHandler(int i, BlockException ex) { public String fooFallbackFunc(int i) { return "eee..."; } + + private String fooFallbackFuncPrivate(int i) { + return "EEE..."; + } + + private String fooBlockHandlerPrivate(int i, BlockException ex) { + return "Oops, " + i; + } }