Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fault tolerance annotations on fallback methods #442

Open
spyrkob opened this issue Jun 21, 2021 · 1 comment
Open

Fault tolerance annotations on fallback methods #442

spyrkob opened this issue Jun 21, 2021 · 1 comment

Comments

@spyrkob
Copy link

spyrkob commented Jun 21, 2021

Fault tolerance annotations are silently ignored on fallback methods. For example:

   @Fallback(fallbackMethod = "legacyFindBook")
   public Book findBookInOnlineStore(String title) {
      ...
   }

   @Bulkhead(value = 1)
   public Book legacyFindBook(String title) {
      ...
   }

The @Bulkhead on legacyFindBook will be ignored and if there are concurrent calls to findBookInOnlineStore multiple threads will be able to execute the fallback method.

If this is planned maybe it would be useful to print a warning in such cases?

@Ladicek
Copy link
Contributor

Ladicek commented Jun 21, 2021

They are not "silently ignored", because there's nothing in the MicroProfile Fault Tolerance specification saying that this should work. More specifically, the fault tolerance annotations are interceptor binding annotations, and interceptors are only applied for business method invocations (see https://docs.jboss.org/cdi/spec/2.0/cdi-spec.html#biz_method). The MP FT specification would have to explicitly say that invocations of fallback methods are business method invocations -- which it doesn't.

I guess the reason is simple: @Fallback is a mechanism for providing, well, fallback values.

There seems to be a growing tendency of trying to coerce @Fallback into a generic exception handling mechanism. That's not its purpose, and I'm very reluctant to add support for anything that would move in that direction.

What you're attempting here falls into that bucket too.

Here's a simple fix:

// you might want to still have a `@Fallback` here, because
// `legacyFindBook` will throw in case of concurrent access
public Book findBookInOnlineStore(String title) {
    try {
        ...
    } catch (Exception e) {
        return legacyFindBook(title);
    }
}

@Bulkhead(value = 1)
public Book legacyFindBook(String title) {
    ...
}

This will work fine in environments that support self-interception, such as Quarkus. In environments that don't, such as WildFly, you'd have to use self-injection.

Printing a warning for fallback methods that have FT annotations is certainly an option. I guess fallback methods shouldn't be called from anywhere else, so such warning is probably safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants