OpenFeign / feign-annotation-error-decoder

Apache License 2.0
51 stars 9 forks source link

v1.0.3 does not work with feign-core 10.x.x #11

Closed karlmuscat closed 5 years ago

karlmuscat commented 5 years ago

Upgraded my projects to JDK 11 and as a result I had to upgrade also the Spring Cloud dependencies to the Greenwich.RELEASE. The Greenwich.RELEASE upgrades the feign libraries to 10.1.0, however the feign-annotation-error-decoder, still uses 9.5.0.

The Response class in the latest feign-core library introduced a check that is breaking the ExceptionGenerator in the feign-annotation-error-decoder

The check is: checkState(builder.request != null, "original request is required");

and this is the broken code:

static {
        Map<String,Collection<String>> testHeaders = new HashMap<String, Collection<String>>();
        testHeaders.put("TestHeader", Arrays.asList("header-value"));

        TEST_RESPONSE = Response.builder()
            .status(500)
            .body((Response.Body) null)
            .headers(testHeaders)
            .build();
    }
karlmuscat commented 5 years ago

The issue is replicable through your unit tests, if the parent version is bumped to 10.2.1

Fix is a one-liner in the static block of ExceptionGenerator

saintf commented 5 years ago

Nice! Thanks! Are you ok to make the change, I'll review it and we try to push it out?

On Tue, 19 Feb 2019, 15:51 Karl, notifications@github.com wrote:

The issue is replicable through your unit tests, if the parent version is bumped to 10.2.1

Fix is a one-liner in the static block of ExceptionGenerator

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/OpenFeign/feign-annotation-error-decoder/issues/11#issuecomment-465159670, or mute the thread https://github.com/notifications/unsubscribe-auth/ANOpit2bOZoX27S1gTAfDZ_jyBCXPbeFks5vPA9mgaJpZM4bCzu1 .

karlmuscat commented 5 years ago

Sure. Just tell me where to commit

saintf commented 5 years ago

The usual way to contribute is:

I'll merge it if all good, and then hit the release button that will upload it to maven Central - and job done :)

Does that sound fair?

On Thu, 21 Feb 2019, 09:37 Karl, notifications@github.com wrote:

Sure. Just tell me where to commit

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OpenFeign/feign-annotation-error-decoder/issues/11#issuecomment-465909874, or mute the thread https://github.com/notifications/unsubscribe-auth/ANOpivCPahbhcKVGFB29EihmgZ9xMJX2ks5vPlq0gaJpZM4bCzu1 .

karlmuscat commented 5 years ago

Very :) PR done. Thanks.

saintf commented 5 years ago

released with https://github.com/OpenFeign/feign-annotation-error-decoder/releases/tag/release-1.1.0 - should be in maven-central shortly. was compiled with openjdk8 (as opposed to oraclejdk8 - which is what I expected it to be compiled with - but... should still work. 1.1.1 will be compiled with oraclejdk8).

karlmuscat commented 5 years ago

is it normal that it's still not visible by this time?

https://mvnrepository.com/artifact/io.github.openfeign/feign-annotation-error-decoder

saintf commented 5 years ago

No... I looked for it. Turns out Travis cancelled the build that pushes it. Since then, I've released a new version (1.1.1) and that didngo through and should show up shortly.

Unfortunately... Due to a race condition, the first build was a Java 11 compiled build. If you pull it and it's java11 (and you need java8) let me know - I'll release a 1.1.2 with Java 8 (I've removed the race at build time issue... So it's always java8 that actually gets deployed/built/etc).

Thanks!

On Fri, 1 Mar 2019, 11:29 Karl, notifications@github.com wrote:

is it normal that it's still not visible by this time?

https://mvnrepository.com/artifact/io.github.openfeign/feign-annotation-error-decoder

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/OpenFeign/feign-annotation-error-decoder/issues/11#issuecomment-468619312, or mute the thread https://github.com/notifications/unsubscribe-auth/ANOpiquqnaDtJ8SCiAHyu9VsD-39jkt7ks5vSQDzgaJpZM4bCzu1 .

saintf commented 5 years ago

@karlmuscat I just pushed 1.1.2 - compiled with java 8 (1.1.1 turns out, in maven central, is compiled with java 11 as I just confirmed). There's no difference between those two. It won't "show up" for a few hours in search results since those take time to get updated, but if you pull it into your project it should work now. Thanks, and sorry for the mess!

RajeevRaparthi commented 4 years ago

First check your dependency by using mvn dependency:tree Use the below dependency Management for spring-boot to download the suitable versions.

    <dependencyManagement>
    <dependencies>
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-dependencies</artifactId>
            <version>${spring-cloud.version}</version>
            <type>pom</type>
            <scope>import</scope>
        </dependency>
    </dependencies>
</dependencyManagement>

I am using Java 10, cloud.version is Finchley.SR2 and sprinb-boot:2.2.0 and spring-cloud-starter-openfeign :2.1.2.RELEASE. and this combination worked for me to fix the issue. use inbuilt jars as wihtout version, so that cloud downloads working versions

io.github.openfeign feign-httpclient

Acctual problem was 10.x.x feign-core was not working only io.github.openfeign:feign-core:jar:9.7.0:compile was working