8000 Adds FallbackFactory, allowing access to the cause of a Hystrix fallback by codefromthecrypt · Pull Request #443 · OpenFeign/feign · GitHub
[go: up one dir, main page]

Skip to content

Adds FallbackFactory, allowing access to the cause of a Hystrix fallback#443

Merged
codefromthecrypt merged 1 commit intomasterfrom
fallback-factory
Aug 19, 2016
Merged

Adds FallbackFactory, allowing access to the cause of a Hystrix fallback#443
codefromthecrypt merged 1 commit intomasterfrom
fallback-factory

Conversation

@codefromthecrypt
Copy link
Contributor

The cause of the fallback is now logged by default to FINE level. You can programmatically inspect
the cause by making your own FallbackFactory. In many cases, the cause will be a FeignException,
which includes the http status.

Here's an example of using FallbackFactory:

// This instance will be invoked if there are errors of any kind.
FallbackFactory<GitHub> fallbackFactory = cause -> (owner, repo) -> {
  if (cause instanceof FeignException && ((FeignException) cause).status() == 403) {
    return Collections.emptyList();
  } else {
    return Arrays.asList("yogi");
  }
};

GitHub github = HystrixFeign.builder()
                            ...
                            .target(GitHub.class, "https://api.github.com", fallbackFactory);

Fixes #431
See #432

@codefromthecrypt
Copy link
8000
Contributor Author

return super.getFallback();
}
try {
Object fallback = fallbackFactory.create(getFailedExecutionException());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: if all we want to do is log the cause, we can delete the entire pull request and just log here!

Throwable cause = getFailedExecutionException();
 if (logger.isLoggable(Level.FINE)) {
  logger.log(Level.FINE, "fallback due to: " + cause.getMessage(), cause);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think folks want access to the actual exception object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okie dokie, then we don't need to delete this branch :)

@codefromthecrypt
Copy link
Contributor Author

PS for those who have existing fallback classes, here's the process for teaching them about the cause..

  1. make a Throwable constructor parameter in your fallback class, and assign it to a field
  2. make a FallbackFactory which invokes that constructor
  3. For any of your methods that care about the cause, read the field you just added.

@ryanjbaxter
Copy link

At first i was slightly confused by the name FallbackFactory but after thinking about it, I guess it makes sense since we are producing a dynamic fallback of sorts. I think this looks good.

@codefromthecrypt
Copy link
Contributor Author
codefromthecrypt commented Aug 17, 2016

added a bunch of example tests, which aren't really about feign, more about a million ways to construct objects. One in particular could be helpful for those who want to retrofit an existing class and not declare a separate FallbackFactory. Note: if feign was java8 we wouldn't even need FallbackFactory (since we could use the Function type)!

  // retrofit so people don't have to track 2 classes
  static class FallbackApiRetro implements Api, FallbackFactory<FallbackApiRetro> {

    @Override public FallbackApiRetro create(Throwable cause) {
      return new FallbackApiRetro(cause);
    }

    final Throwable cause; // nullable

    public FallbackApiRetro() {
      this(null);
    }

    FallbackApiRetro(Throwable cause) {
      this.cause = cause;
    }

    // actual api methods can lazy implement any use of the cause.
    @Override public String invoke() {
      return cause != null ? cause.getMessage() : "foo";
    }
  }

@codefromthecrypt codefromthecrypt force-pushed the fallback-factory branch 2 times, most recently from a364b23 to 4457aea Compare August 17, 2016 07:30
@codefromthecrypt
Copy link
Contributor Author

ps if we aren't quite ready to land this PR, we should probably go ahead and do a release. If that's the case, I can add a logging line as an interim step. Lemme know.

@bitsofinfo
Copy link

+1 I definitely have a need for this, right now when a fallback is invoked I have no idea about the underlying reason

@bpicode
Copy link
bpicode commented Aug 19, 2016

+1 this feature is a must

The cause of the fallback is now logged by default to FINE level. You can programmatically inspect
the cause by making your own `FallbackFactory`. In many cases, the cause will be a `FeignException`,
which includes the http status.

Here's an example of using `FallbackFactory`:

```java
// This instance will be invoked if there are errors of any kind.
FallbackFactory<GitHub> fallbackFactory = cause -> (owner, repo) -> {
  if (cause instanceof FeignException && ((FeignException) cause).status() == 403) {
    return Collections.emptyList();
  } else {
    return Arrays.asList("yogi");
  }
};

GitHub github = HystrixFeign.builder()
                            ...
                            .target(GitHub.class, "https://api.github.com", fallbackFactory);
```
@codefromthecrypt
Copy link
Contributor Author

OK will merge on green then cut feign 9.3

@codefromthecrypt codefromthecrypt merged commit 8ecb6ad into master Aug 19, 2016
@codefromthecrypt codefromthecrypt deleted the fallback-factory branch August 19, 2016 12:48
@seetharamani
Copy link

Hi is this Feign FallbackFactory is working? I am using version 9.3.1. I am still not able to get the cause from fallback. I used the code example from retrofit test case.

@spencergibb
Copy link
Contributor

@seetharamani there are tests that demonstrate it working. If you think you have found a bug, open an issue with steps to reproduce or a sample project that demonstrates the problem. Asking if it works on the merge PR isn't real helpful.

@spencergibb
Copy link
Contributor

@seetharamani

open an issue with steps to reproduce or a sample project that demonstrates the problem

@seetharamani
Copy link

Opened the issue with sample project
#458

velo pushed a commit that referenced this pull request Oct 7, 2024
…ack (#443)

The cause of the fallback is now logged by default to FINE level. You can programmatically inspect
the cause by making your own `FallbackFactory`. In many cases, the cause will be a `FeignException`,
which includes the http status.

Here's an example of using `FallbackFactory`:

```java
// This instance will be invoked if there are errors of any kind.
FallbackFactory<GitHub> fallbackFactory = cause -> (owner, repo) -> {
  if (cause instanceof FeignException && ((FeignException) cause).status() == 403) {
    return Collections.emptyList();
  } else {
    return Arrays.asList("yogi");
  }
};

GitHub github = HystrixFeign.builder()
                            ...
                            .target(GitHub.class, "https://api.github.com", fallbackFactory);
```
velo pushed a commit that referenced this pull request Oct 8, 2024
…ack (#443)

The cause of the fallback is now logged by default to FINE level. You can programmatically inspect
the cause by making your own `FallbackFactory`. In many cases, the cause will be a `FeignException`,
which includes the http status.

Here's an example of using `FallbackFactory`:

```java
// This instance will be invoked if there are errors of any kind.
FallbackFactory<GitHub> fallbackFactory = cause -> (owner, repo) -> {
  if (cause instanceof FeignException && ((FeignException) cause).status() == 403) {
    return Collections.emptyList();
  } else {
    return Arrays.asList("yogi");
  }
};

GitHub github = HystrixFeign.builder()
                            ...
                            .target(GitHub.class, "https://api.github.com", fallbackFactory);
```
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

Successfully merging this pull request may close these issues.

6 participants

0