Skip to content

Commit 982ee99

Browse files
jvanderhoekvelo
jvanderhoek
authored and
velo
committed
Using proper formatting pattern for logIOException (OpenFeign#614)
* Using proper formatting pattern for logIOException. Fixes OpenFeign#613 * Enforced rule exection order for LoggerTest such that ExpectedException is caught before the other rules. (The recording logging rule was ignored for tests that throw an exception). Fixed up test cases that never ran because of this. Added test for format character bug.
1 parent 47b7aa2 commit 982ee99

2 files changed

Lines changed: 81 additions & 13 deletions

File tree

‎core/src/main/java/feign/Logger.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ protected IOException logIOException(String configKey, Level logLevel, IOExcepti
114114
if (logLevel.ordinal() >= Level.FULL.ordinal()) {
115115
StringWriter sw = new StringWriter();
116116
ioe.printStackTrace(new PrintWriter(sw));
117-
log(configKey, sw.toString());
117+
log(configKey, "%s", sw.toString());
118118
log(configKey, "<--- END ERROR");
119119
}
120120
return ioe;

‎core/src/test/java/feign/LoggerTest.java‎

Lines changed: 80 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.junit.Test;
2222
import org.junit.experimental.runners.Enclosed;
2323
import org.junit.rules.ExpectedException;
24+
import org.junit.rules.RuleChain;
2425
import org.junit.rules.TestRule;
2526
import org.junit.runner.Description;
2627
import org.junit.runner.RunWith;
@@ -39,12 +40,14 @@
3940
@RunWith(Enclosed.class)
4041
public class LoggerTest {
4142

42-
@Rule
43+
public final ExpectedException thrown = ExpectedException.none();
4344
public final MockWebServer server = new MockWebServer();
44-
@Rule
4545
public final RecordingLogger logger = new RecordingLogger();
46+
47+
/** Ensure expected exception handling is done before logger rule. */
4648
@Rule
47-
public final ExpectedException thrown = ExpectedException.none();
49+
public final RuleChain chain= RuleChain.outerRule( server ).around( logger ).around( thrown );
50+
4851

4952
interface SendsStuff {
5053

@@ -157,15 +160,12 @@ public static Iterable<Object[]> data() {
157160
{Level.NONE, Arrays.asList()},
158161
{Level.BASIC, Arrays.asList(
159162
"\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
160-
"\\[SendsStuff#login\\] <--- HTTP/1.1 200 OK \\([0-9]+ms\\)",
161163
"\\[SendsStuff#login\\] <--- ERROR SocketTimeoutException: Read timed out \\([0-9]+ms\\)")},
162164
{Level.HEADERS, Arrays.asList(
163165
"\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
164166
"\\[SendsStuff#login\\] Content-Type: application/json",
165167
"\\[SendsStuff#login\\] Content-Length: 80",
166168
"\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
167-
"\\[SendsStuff#login\\] <--- HTTP/1.1 200 OK \\([0-9]+ms\\)",
168-
"\\[SendsStuff#login\\] content-length: 3",
169169
"\\[SendsStuff#login\\] <--- ERROR SocketTimeoutException: Read timed out \\([0-9]+ms\\)")},
170170
{Level.FULL, Arrays.asList(
171171
"\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
@@ -174,11 +174,8 @@ public static Iterable<Object[]> data() {
174174
"\\[SendsStuff#login\\] ",
175175
"\\[SendsStuff#login\\] \\{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"\\}",
176176
"\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
177-
"\\[SendsStuff#login\\] <--- HTTP/1.1 200 OK \\([0-9]+ms\\)",
178-
"\\[SendsStuff#login\\] content-length: 3",
179-
"\\[SendsStuff#login\\] ",
180177
"\\[SendsStuff#login\\] <--- ERROR SocketTimeoutException: Read timed out \\([0-9]+ms\\)",
181-
"\\[SendsStuff#login\\] java.net.SocketTimeoutException: Read timed out.*",
178+
"(?s)\\[SendsStuff#login\\] java.net.SocketTimeoutException: Read timed out.*",
182179
"\\[SendsStuff#login\\] <--- END ERROR")}
183180
});
184181
}
@@ -192,6 +189,15 @@ public void levelEmitsOnReadTimeout() throws IOException, InterruptedException {
192189
.logger(logger)
193190
.logLevel(logLevel)
194191
.options(new Request.Options(10 * 1000, 50))
192+
.retryer(new Retryer() {
193+
@Override
194+
public void continueOrPropagate(RetryableException e) {
195+
throw e;
196+
}
197+
@Override public Retryer clone() {
198+
return this;
199+
}
200+
})
195201
.target(SendsStuff.class, "http://localhost:" + server.getPort());
196202

197203
api.login("netflix", "denominator", "password");
@@ -229,7 +235,7 @@ public static Iterable<Object[]> data() {
229235
"\\[SendsStuff#login\\] \\{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"\\}",
230236
"\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
231237
"\\[SendsStuff#login\\] <--- ERROR UnknownHostException: robofu.abc \\([0-9]+ms\\)",
232-
"\\[SendsStuff#login\\] java.net.UnknownHostException: robofu.abc.*",
238+
"(?s)\\[SendsStuff#login\\] java.net.UnknownHostException: robofu.abc.*",
233239
"\\[SendsStuff#login\\] <--- END ERROR")}
234240
});
235241
}
@@ -256,6 +262,67 @@ public void continueOrPropagate(RetryableException e) {
256262
}
257263
}
258264

265+
266+
@RunWith(Parameterized.class)
267+
public static class FormatCharacterTest
268+
extends LoggerTest {
269+
270+
private final Level logLevel;
271+
272+
public FormatCharacterTest( Level logLevel, List<String> expectedMessages) {
273+
this.logLevel = logLevel;
274+
logger.expectMessages(expectedMessages);
275+
}
276+
277+
@Parameters
278+
public static Iterable<Object[]> data() {
279+
return Arrays.asList(new Object[][]{
280+
{Level.NONE, Arrays.asList()},
281+
{Level.BASIC, Arrays.asList(
282+
"\\[SendsStuff#login\\] ---> POST http://sna%fu.abc/ HTTP/1.1",
283+
"\\[SendsStuff#login\\] <--- ERROR UnknownHostException: sna%fu.abc \\([0-9]+ms\\)")},
284+
{Level.HEADERS, Arrays.asList(
285+
"\\[SendsStuff#login\\] ---> POST http://sna%fu.abc/ HTTP/1.1",
286+
"\\[SendsStuff#login\\] Content-Type: application/json",
287+
"\\[SendsStuff#login\\] Content-Length: 80",
288+
"\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
289+
"\\[SendsStuff#login\\] <--- ERROR UnknownHostException: sna%fu.abc \\([0-9]+ms\\)")},
290+
{Level.FULL, Arrays.asList(
291+
"\\[SendsStuff#login\\] ---> POST http://sna%fu.abc/ HTTP/1.1",
292+
"\\[SendsStuff#login\\] Content-Type: application/json",
293+
"\\[SendsStuff#login\\] Content-Length: 80",
294+
"\\[SendsStuff#login\\] ",
295+
"\\[SendsStuff#login\\] \\{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"\\}",
296+
"\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
297+
"\\[SendsStuff#login\\] <--- ERROR UnknownHostException: sna%fu.abc \\([0-9]+ms\\)",
298+
"(?s)\\[SendsStuff#login\\] java.net.UnknownHostException: sna%fu.abc.*",
299+
"\\[SendsStuff#login\\] <--- END ERROR")}
300+
});
301+
}
302+
303+
@Test
304+
public void formatCharacterEmits() throws IOException, InterruptedException {
305+
SendsStuff api = Feign.builder()
306+
.logger(logger)
307+
.logLevel(logLevel)
308+
.retryer(new Retryer() {
309+
@Override
310+
public void continueOrPropagate(RetryableException e) {
311+
throw e;
312+
}
313+
@Override public Retryer clone() {
314+
return this;
315+
}
316+
})
317+
.target(SendsStuff.class, "http://sna%fu.abc");
318+
319+
thrown.expect(FeignException.class);
320+
321+
api.login("netflix", "denominator", "password");
322+
}
323+
}
324+
325+
259326
@RunWith(Parameterized.class)
260327
public static class RetryEmitsTest extends LoggerTest {
261328

@@ -331,7 +398,8 @@ public Statement apply(final Statement base, Description description) {
331398
public void evaluate() throws Throwable {
332399
base.evaluate();
333400
SoftAssertions softly = new SoftAssertions();
334-
for (int i = 0; i < messages.size(); i++) {
401+
softly.assertThat( messages.size() ).isEqualTo( expectedMessages.size() );
402+
for (int i = 0; i < messages.size() && i<expectedMessages.size(); i++) {
335403
softly.assertThat(messages.get(i)).matches(expectedMessages.get(i));
336404
}
337405
softly.assertAll();

0 commit comments

Comments
 (0)