Skip to content

Commit

Permalink
Improve logging and fix CI (#21)
Browse files Browse the repository at this point in the history
  • Loading branch information
jebeaudet authored Apr 26, 2023
1 parent 28fa8fb commit b5375db
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 43 deletions.
33 changes: 33 additions & 0 deletions .github/workflows/maven.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: Java CI with Maven

on:
push:
branches: [ "main" ]
pull_request:
branches: [ "main" ]

jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Set up JDK 8
uses: actions/setup-java@v3
with:
java-version: '8'
distribution: 'temurin'
cache: maven
- name: Build with Maven with jdk 8
run: mvn -ntp clean compile test-compile --file pom.xml
- name: Run tests with jdk 8
run: mvn -ntp -fae test --file pom.xml
- name: Set up JDK 17
uses: actions/setup-java@v3
with:
java-version: '17'
distribution: 'temurin'
cache: maven
- name: Build with Maven with jdk 17
run: mvn -ntp clean compile test-compile --file pom.xml
- name: Run tests with jdk 17
run: mvn -fae -ntp test --file pom.xml
8 changes: 0 additions & 8 deletions .travis.yml

This file was deleted.

3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
[![Build Status](https://travis-ci.org/coveo/feign-error-decoder.svg?branch=master)](https://travis-ci.org/coveo/feign-error-decoder)
[![MIT license](http://img.shields.io/badge/license-MIT-brightgreen.svg)](https://github.com/coveo/feign-error-decoder/blob/master/LICENSE)
[![Maven Central](https://maven-badges.herokuapp.com/maven-central/com.coveo/feign-error-decoder/badge.svg)](https://maven-badges.herokuapp.com/maven-central/com.coveo/feign-error-decoder)

Expand All @@ -17,7 +16,7 @@ The plugin is available on Maven Central :
<dependency>
<groupId>com.coveo</groupId>
<artifactId>feign-error-decoder</artifactId>
<version>2.0.0</version>
<version>2.2.0</version>
</dependency>
```

Expand Down
20 changes: 18 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>com.coveo</groupId>
<artifactId>feign-error-decoder</artifactId>
<version>2.1.0</version>
<version>2.2.0</version>
<packaging>jar</packaging>

<name>Feign Reflection Error Decoder</name>
Expand Down Expand Up @@ -188,7 +188,23 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.0.0-M7</version>
<version>3.0.0</version>
<dependencies>
<dependency>
<groupId>me.fabriciorby</groupId>
<artifactId>maven-surefire-junit5-tree-reporter</artifactId>
<version>1.1.0</version>
</dependency>
</dependencies>
<configuration>
<reportFormat>plain</reportFormat>
<consoleOutputReporter>
<disable>true</disable>
</consoleOutputReporter>
<statelessTestsetInfoReporter
implementation="org.apache.maven.plugin.surefire.extensions.junit5.JUnit5StatelessTestsetInfoTreeReporterUnicode">
</statelessTestsetInfoReporter>
</configuration>
</plugin>
</plugins>
</build>
Expand Down
17 changes: 11 additions & 6 deletions src/main/java/com/coveo/feign/ReflectionErrorDecoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,10 @@ private void initialize() {
detailMessageField = Throwable.class.getDeclaredField("detailMessage");
detailMessageField.setAccessible(true);
} catch (Exception e) {
logger.debug("Unable to directly set the detailMessage, make sure exception do implement {}", baseExceptionClass.getName());
logger.debug(
"Unable to set the detailMessage via reflection, make sure the base exception do implement '{}'. Error message: '{}'.",
ExceptionMessageSetter.class.getName(),
e.getMessage());
detailMessageField = null;
}

Expand Down Expand Up @@ -184,7 +187,7 @@ private RuntimeException getRuntimeExceptionByReflection(String exceptionKey, T
((ExceptionMessageSetter) runtimeExceptionToBeThrown)
.setExceptionMessage(getMessageFromResponse(apiResponse));
} else {
if(detailMessageField != null) {
if (detailMessageField != null) {
detailMessageField.set(runtimeExceptionToBeThrown, getMessageFromResponse(apiResponse));
}
}
Expand All @@ -198,7 +201,7 @@ private S getExceptionByReflection(String exceptionKey, T apiResponse)
((ExceptionMessageSetter) exceptionToBeThrown)
.setExceptionMessage(getMessageFromResponse(apiResponse));
} else {
if(detailMessageField != null) {
if (detailMessageField != null) {
detailMessageField.set(exceptionToBeThrown, getMessageFromResponse(apiResponse));
}
}
Expand Down Expand Up @@ -244,11 +247,13 @@ private void extractExceptionInfo(Class<? extends S> clazz)
existingExceptionDetails.getClazz().getName()));
}

if (!exceptionMessageHandlingLogged
if (detailMessageField == null
&& !exceptionMessageHandlingLogged
&& !ExceptionMessageSetter.class.isAssignableFrom(clazz)) {
logger.warn(
"The class '{}' or its superclass(es) do not implement ExceptionMessageSetter, therefore the Throwable detailMessage field will not be set. This will be only logged once.",
clazz);
"The class '{}' or its superclass(es) do not implement '{}', therefore the Throwable detailMessage field will not be set. This will be only logged once.",
clazz,
ExceptionMessageSetter.class.getName());
exceptionMessageHandlingLogged = true;
}
}
Expand Down
1 change: 0 additions & 1 deletion src/test/java/com/coveo/feign/BaseServiceException.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ protected BaseServiceException(String errorCode, String message, Throwable inner
public String getErrorCode() {
return errorCode;
}

}
32 changes: 18 additions & 14 deletions src/test/java/com/coveo/feign/ReflectionErrorDecoderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,13 @@ public void testDecodeThrownSubAbstractException() throws Exception {
}

@Test
@EnabledForJreRange(max=JRE.JAVA_15)
@EnabledForJreRange(max = JRE.JAVA_15)
public void testDecodeThrownSubAbstractExceptionWithoutInterface() throws Exception {
ServiceExceptionErrorDecoder errorDecoder =
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
Response response =
getResponseWithErrorCode(ConcreteSubServiceExceptionWithoutInterface.ERROR_CODE, DUMMY_MESSAGE);
getResponseWithErrorCode(
ConcreteSubServiceExceptionWithoutInterface.ERROR_CODE, DUMMY_MESSAGE);

Exception exception = errorDecoder.decode("", response);

Expand All @@ -236,12 +237,14 @@ public void testDecodeThrownSubAbstractExceptionWithoutInterface() throws Except
}

@Test
@EnabledForJreRange(min=JRE.JAVA_16)
public void testDecodeThrownSubAbstractExceptionWithoutInterfaceShouldDoNothing() throws Exception {
@EnabledForJreRange(min = JRE.JAVA_16)
public void testDecodeThrownSubAbstractExceptionWithoutInterfaceShouldDoNothing()
throws Exception {
ServiceExceptionErrorDecoder errorDecoder =
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
Response response =
getResponseWithErrorCode(ConcreteSubServiceExceptionWithoutInterface.ERROR_CODE, DUMMY_MESSAGE);
getResponseWithErrorCode(
ConcreteSubServiceExceptionWithoutInterface.ERROR_CODE, DUMMY_MESSAGE);

Exception exception = errorDecoder.decode("", response);

Expand Down Expand Up @@ -293,12 +296,12 @@ public void testAdditionalRuntimeException() throws Exception {
}

@Test
@EnabledForJreRange(max=JRE.JAVA_15)
@EnabledForJreRange(max = JRE.JAVA_15)
public void testAdditionalRuntimeExceptionWithoutInterface() throws Exception {
ServiceExceptionErrorDecoder errorDecoder =
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
Response response =
getResponseWithErrorCode(AdditionalNotInterfacedRuntimeException.ERROR_CODE, DUMMY_MESSAGE);
getResponseWithErrorCode(AdditionalNotInterfacedRuntimeException.ERROR_CODE, DUMMY_MESSAGE);

Exception exception = errorDecoder.decode("", response);

Expand All @@ -307,17 +310,18 @@ public void testAdditionalRuntimeExceptionWithoutInterface() throws Exception {
}

@Test
@EnabledForJreRange(min=JRE.JAVA_16)
@EnabledForJreRange(min = JRE.JAVA_16)
public void testAdditionalRuntimeExceptionWithoutInterfaceShouldDoNothing() throws Exception {
ServiceExceptionErrorDecoder errorDecoder =
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
new ServiceExceptionErrorDecoder(TestApiClassWithPlainExceptions.class);
Response response =
getResponseWithErrorCode(AdditionalNotInterfacedRuntimeException.ERROR_CODE, DUMMY_MESSAGE);
getResponseWithErrorCode(AdditionalNotInterfacedRuntimeException.ERROR_CODE, DUMMY_MESSAGE);

Exception exception = errorDecoder.decode("", response);

assertThat(exception).isInstanceOf(AdditionalNotInterfacedRuntimeException.class);
assertThat(exception.getMessage()).isEqualTo(AdditionalNotInterfacedRuntimeException.ERROR_MESSAGE);
assertThat(exception.getMessage())
.isEqualTo(AdditionalNotInterfacedRuntimeException.ERROR_MESSAGE);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ public void setExceptionMessage(String detailMessage) {
}
}

public static class AdditionalNotInterfacedRuntimeException extends AbstractAdditionalRuntimeException {
public static class AdditionalNotInterfacedRuntimeException
extends AbstractAdditionalRuntimeException {
private static final long serialVersionUID = 1L;
public static final String ERROR_CODE = "DOYOUEVENIMPLEMENTBRO?";
public static final String ERROR_MESSAGE = "PANICATTHEDISCO";
Expand All @@ -169,7 +170,6 @@ public AdditionalNotInterfacedRuntimeException() {
public String getMessage() {
return detailMessage == null ? super.getMessage() : detailMessage;
}

}

public abstract static class AbstractAdditionalRuntimeException extends RuntimeException {
Expand Down Expand Up @@ -292,9 +292,13 @@ public ConcreteSubServiceException() {
}
}

public static class ConcreteSubServiceExceptionWithoutInterface extends ServiceExceptionWithoutInterface {
public static class ConcreteSubServiceExceptionWithoutInterface
extends ServiceExceptionWithoutInterface {
public static final String ERROR_CODE = "I WISH I HAD A PROPER INTERFACE";
public ConcreteSubServiceExceptionWithoutInterface() { super(ERROR_CODE);}

public ConcreteSubServiceExceptionWithoutInterface() {
super(ERROR_CODE);
}
}

public static class DuplicateErrorCodeServiceException extends ServiceException {
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/com/coveo/feign/ServiceException.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

import com.coveo.feign.annotation.ExceptionMessageSetter;

public abstract class ServiceException extends BaseServiceException implements ExceptionMessageSetter {
public abstract class ServiceException extends BaseServiceException
implements ExceptionMessageSetter {
private static final long serialVersionUID = 4116691862956368612L;
private String detailMessage;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ protected void addAdditionalRuntimeExceptions(
.withClazz(AdditionalRuntimeException.class)
.withExceptionSupplier(AdditionalRuntimeException::new));
runtimeExceptionsThrown.put(
AdditionalNotInterfacedRuntimeException.ERROR_CODE,
new ThrownExceptionDetails<RuntimeException>()
.withClazz(AdditionalNotInterfacedRuntimeException.class)
.withExceptionSupplier(AdditionalNotInterfacedRuntimeException::new));
AdditionalNotInterfacedRuntimeException.ERROR_CODE,
new ThrownExceptionDetails<RuntimeException>()
.withClazz(AdditionalNotInterfacedRuntimeException.class)
.withExceptionSupplier(AdditionalNotInterfacedRuntimeException::new));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.coveo.feign;


public abstract class ServiceExceptionWithoutInterface extends BaseServiceException {
private static final long serialVersionUID = 4116691862956368612L;

Expand Down

0 comments on commit b5375db

Please sign in to comment.