Commit 78bbfaea authored by Georg Mittendorfer's avatar Georg Mittendorfer

Improve retry log. Retry for inconsistent tips.

parent 76306a59
......@@ -21,6 +21,8 @@ package com.mio.piri.commands;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.mio.piri.commands.response.GetBalancesErrorHandler;
import com.mio.piri.commands.response.ResponseErrorHandler;
import lombok.Getter;
import lombok.Setter;
import lombok.ToString;
......@@ -36,6 +38,9 @@ import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
@ToString
public class GetBalances extends IriCommand {
@JsonIgnore
private static final ResponseErrorHandler ERROR_HANDLER = new GetBalancesErrorHandler();
// @NotEmpty can be empty for a new seed (light wallet)
@NotNull
@JsonInclude(value=NON_NULL)
......@@ -53,4 +58,9 @@ public class GetBalances extends IriCommand {
return CollectionUtils.size(addresses);
}
@JsonIgnore
public ResponseErrorHandler getResponseErrorHandler() {
return GetBalances.ERROR_HANDLER;
}
}
......@@ -24,7 +24,7 @@ import org.springframework.http.HttpStatus;
public class DefaultErrorHandler implements ResponseErrorHandler {
@Override
public boolean isCriticalError(HttpStatus httpStatus) {
public boolean isCriticalError(HttpStatus httpStatus, String body) {
return httpStatus.isError() && httpStatus != HttpStatus.BAD_REQUEST;
}
......
/*
* Copyright (c) 2019.
*
* This file is part of Piri.
*
* Piri is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published
* by the Free Software Foundation, either version 3 of the License,
* or (at your option) any later version.
*
* Piri is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
* the GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public
* License along with Piri. If not, see <http://www.gnu.org/licenses/>.
*/
package com.mio.piri.commands.response;
import org.apache.commons.lang3.StringUtils;
import org.springframework.http.HttpStatus;
public class GetBalancesErrorHandler extends DefaultErrorHandler {
@Override
public boolean isCriticalError(HttpStatus httpStatus, String body) {
return super.isCriticalError(httpStatus, body)
|| (httpStatus == HttpStatus.BAD_REQUEST && StringUtils.containsIgnoreCase(body, "Tips are not consistent"));
}
}
......@@ -23,6 +23,6 @@ import org.springframework.http.HttpStatus;
public interface ResponseErrorHandler {
boolean isCriticalError(HttpStatus httpStatus);
boolean isCriticalError(HttpStatus httpStatus, String body);
}
......@@ -48,14 +48,17 @@ import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.web.bind.WebDataBinder;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.bind.annotation.CrossOrigin;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.InitBinder;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.support.WebExchangeBindException;
import org.springframework.web.reactive.function.client.WebClientResponseException;
import org.springframework.web.server.ResponseStatusException;
import reactor.core.publisher.Mono;
import javax.validation.Valid;
import java.nio.charset.Charset;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
......@@ -110,6 +113,7 @@ public class IriApiHandler {
command.setIp(ip);
command.setUserAgent(StringUtils.abbreviate(session.getUserAgent(request), 64));
// TODO use defer and retry
Mono<ResponseEntity<String>> responseEntityMono = executeCommand(command)
.onErrorResume(retryPredicate(), throwable -> retry(throwable, command))
.onErrorResume(retryPredicate(), throwable -> retry(throwable, command))
......@@ -144,7 +148,7 @@ public class IriApiHandler {
private Mono<ResponseEntity<String>> retry(Throwable t, IriCommand command) {
failedCommandCounter(command.getCommand(), t).increment();
logger.info("Retry: {}", StringUtils.abbreviate(command.toString(), 100));
logger.info("Retry #{} for client [{}]: {}", command.getExecutionHistory().size(), command.getIp(), StringUtils.abbreviate(command.toString(), 100));
if (logger.isTraceEnabled()) {
logger.trace(t.toString(), t);
}
......@@ -175,10 +179,8 @@ public class IriApiHandler {
HttpStatus httpStatus = responseEntity.getStatusCode();
// TODO add error handling for unsynced get node info
if (httpStatus.isError()) {
if (command.getResponseErrorHandler().isCriticalError(httpStatus)) {
throw WebClientResponseException.create(httpStatus.value(), httpStatus.getReasonPhrase(), responseEntity.getHeaders(),
responseEntity.getBody() != null ? responseEntity.getBody().getBytes() : null,
Charset.forName("UTF-8"));
if (command.getResponseErrorHandler().isCriticalError(httpStatus, responseEntity.getBody())) {
throw new ResponseStatusException(httpStatus, responseEntity.getBody());
}
} else {
// successful requests might count multiple times. this is a workaround for not being able
......
......@@ -26,22 +26,22 @@ import static org.assertj.core.api.Assertions.assertThat;
public class DefaultErrorHandlerTest {
private final ResponseErrorHandler errorHandler = new DefaultErrorHandler();
private final DefaultErrorHandler errorHandler = new DefaultErrorHandler();
@Test
public void givenNoErrorWhenIsCriticalErrorThenFalse() {
assertThat(errorHandler.isCriticalError(HttpStatus.OK)).isFalse();
assertThat(errorHandler.isCriticalError(HttpStatus.BAD_REQUEST)).isFalse();
assertThat(errorHandler.isCriticalError(HttpStatus.OK, null)).isFalse();
assertThat(errorHandler.isCriticalError(HttpStatus.BAD_REQUEST, null)).isFalse();
}
@Test
public void givenErrorWhenIsCriticalErrorThenTrue() {
assertThat(errorHandler.isCriticalError(HttpStatus.TOO_MANY_REQUESTS)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.NOT_FOUND)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.FORBIDDEN)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.UNAUTHORIZED)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.INTERNAL_SERVER_ERROR)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.SERVICE_UNAVAILABLE)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.TOO_MANY_REQUESTS, null)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.NOT_FOUND, null)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.FORBIDDEN, null)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.UNAUTHORIZED, null)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.INTERNAL_SERVER_ERROR, null)).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.SERVICE_UNAVAILABLE, null)).isTrue();
}
}
\ No newline at end of file
/*
* Copyright (c) 2019.
*
* This file is part of Piri.
*
* Piri is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published
* by the Free Software Foundation, either version 3 of the License,
* or (at your option) any later version.
*
* Piri is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
* the GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public
* License along with Piri. If not, see <http://www.gnu.org/licenses/>.
*/
package com.mio.piri.commands.response;
import org.junit.Test;
import org.springframework.http.HttpStatus;
import static org.assertj.core.api.Assertions.assertThat;
public class GetBalancesErrorHandlerTest {
private final GetBalancesErrorHandler errorHandler = new GetBalancesErrorHandler();
@Test
public void givenNoErrorWhenIsCriticalErrorThenFalse() {
assertThat(errorHandler.isCriticalError(HttpStatus.OK, null)).isFalse();
assertThat(errorHandler.isCriticalError(HttpStatus.BAD_REQUEST, null)).isFalse();
}
@Test
public void givenErrorWhenIsCriticalErrorThenTrue() {
assertThat(errorHandler.isCriticalError(HttpStatus.BAD_REQUEST, "Tips are not consistent.")).isTrue();
assertThat(errorHandler.isCriticalError(HttpStatus.INTERNAL_SERVER_ERROR, null)).isTrue();
}
}
\ No newline at end of file
......@@ -21,12 +21,15 @@ package com.mio.piri.service;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.mio.piri.commands.*;
import com.mio.piri.commands.AttachToTangle;
import com.mio.piri.commands.GetBalances;
import com.mio.piri.commands.GetTransactionsToApprove;
import com.mio.piri.commands.IriCommand;
import com.mio.piri.commands.response.NodeInfo;
import com.mio.piri.metrics.MeterFactory;
import com.mio.piri.util.IriClientTestUtility;
import com.mio.piri.util.responses.ResponseHashes;
import com.mio.piri.util.responses.GetTransactionsToApproveResponse;
import com.mio.piri.util.responses.ResponseHashes;
import com.mio.piri.util.responses.ResponseTrytes;
import io.micrometer.core.instrument.Timer;
import org.assertj.core.api.Assertions;
......@@ -41,13 +44,12 @@ import org.springframework.http.MediaType;
import org.springframework.test.context.junit4.SpringRunner;
import org.springframework.test.web.reactive.server.WebTestClient;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;
import static com.mio.piri.commands.IriCommands.GET_NODE_INFO;
import static com.mio.piri.util.TestCommandFactory.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@SuppressWarnings("SpellCheckingInspection")
@RunWith(SpringRunner.class)
......@@ -103,13 +105,13 @@ public class IriApiHandlerCommandsSysIT {
@Test
public void givenGetNodeInfoWhenPostThenReturnIriNodeInfo() {
String result = postCommandExpectOk(command(GET_NODE_INFO.getKey()));
assertTrue(result, result.startsWith("{\"appName\":\"IRI\""));
assertThat(result).startsWith("{\"appName\":\"IRI\"");
}
@Test
public void givenGetTipsWhenPostThenReturnTips() {
String result = postCommandExpectOk(command("getTips"));
assertTrue(result, result.startsWith("{\"hashes\":[\""));
assertThat(result).startsWith("{\"hashes\":[\"");
}
@Test
......@@ -123,7 +125,7 @@ public class IriApiHandlerCommandsSysIT {
@Test
public void givenGetTrytesWhenPostThenReturnHashes() {
String result = postCommandExpectOk(getTrytes("BDHIGZXHYDNTVQVNIRYYLNPMZITPOUXBJPHCVXYTTURNR9XGAVKMAAJARQPFYPLWEXKEHXYAVLOE99999"));
assertTrue(result, result.startsWith("{\"trytes\":[\""));
assertThat(result).startsWith("{\"trytes\":[\"");
}
@Test
......@@ -132,14 +134,27 @@ public class IriApiHandlerCommandsSysIT {
assert tips != null && tips.getHashes() != null;
String tx = tips.getHashes().get(0);
String result = postCommandExpectOk(getInclusionStates(tx, tx));
assertTrue(result, result.startsWith("{\"states\":[true]")); // same tx naturally includes itself
assertThat(result).startsWith("{\"states\":[true]"); // same tx naturally includes itself
}
// @Test
// public void givenGetBalancesWhenPostThenReturnBalances() {
// String address = "NAFAEIOFEAIJKKDMDVDEXKCDBIMSHIAFSMURU9WDFRVPKEZX9AKGYYOJSXKOAXRPNPJTDDRWXFRADKEDC";
// String result = postCommandExpectOk(getBalances(address));
// assertThat(result).startsWith("{\"balances\":[");
// }
@Test
public void givenGetBalancesWhenPostThenReturnBalances() {
String address = "NAFAEIOFEAIJKKDMDVDEXKCDBIMSHIAFSMURU9WDFRVPKEZX9AKGYYOJSXKOAXRPNPJTDDRWXFRADKEDC";
String result = postCommandExpectOk(getBalances(address));
assertTrue(result, result.startsWith("{\"balances\":["));
GetBalances getBalances = getBalances();
String[] addresses = {
"HKRAKXCWXMYGP9IADTSVUMKPW9HYICJNDDTBPTPJF9XBEEHFQAMWLXMNGUBRDAFOKFYRDWGPPZV9SLLBC",
"RHFDHFAJNHVKPTFEBRVZMRKAVTOKTUYGRXPI99XZBZNQ9CGKVXBHALGIJOTELBHPFQWZEDSBGCWWXNJLY",
};
getBalances.setAddresses(Arrays.asList(addresses));
String result = postCommandExpectOk(getBalances);
assertThat(result).startsWith("{\"balances\":[");
logger.info(result);
}
@Test
......@@ -163,21 +178,21 @@ public class IriApiHandlerCommandsSysIT {
// .expectBody(String.class).returnResult().getResponseBody();
// logger.debug(result);
// assertNotNull(result);
// assertTrue(result, result.startsWith("{\"trytes\":"));
// assertThat(result).startsWith("{\"trytes\":"));
// }
String result = util.postCommand("/", att, 45)
.expectStatus().isOk()
.expectBody(String.class).returnResult().getResponseBody();
logger.debug(result);
assertNotNull(result);
assertTrue(result, result.startsWith("{\"trytes\":"));
assertThat(result).isNotNull();
assertThat(result).startsWith("{\"trytes\":");
}
@Test
public void givenInterruptAttachingToTangleWhenPostThenOk() {
String result = postCommandExpectOk(command("interruptAttachingToTangle"));
assertTrue(result, result.contains("duration"));
assertThat(result).contains("duration");
}
@Test
......@@ -186,7 +201,7 @@ public class IriApiHandlerCommandsSysIT {
ResponseTrytes responseTrytes = postCommandExpectOk(getTrytes(nodeInfo.getLatestSolidSubtangleMilestone()), ResponseTrytes.class); // convert to trytes
String trytes = responseTrytes.getTrytes().get(0);
String result = postCommandExpectOk(broadcastTransactions(trytes));
assertTrue(result, result.contains("duration"));
assertThat(result).contains("duration");
}
@Test
......@@ -195,46 +210,46 @@ public class IriApiHandlerCommandsSysIT {
ResponseTrytes responseTrytes = postCommandExpectOk(getTrytes(nodeInfo.getLatestSolidSubtangleMilestone()), ResponseTrytes.class); // convert to trytes
String trytes = responseTrytes.getTrytes().get(0);
String result = postCommandExpectOk(storeTransactions(trytes));
assertTrue(result, result.contains("duration"));
assertThat(result).contains("duration");
}
@Test
public void givenWereAddressesSpentFromWhenPostThenReturnStates() {
String address = "UIQJD9ADKNZYSL9CCJULJQNWTZ9FWGVQMUFSXDK9SJVINWQOVGKXLMSNWJGCCMAQRUTSETKUSCFXGMTHA";
String result = postCommandExpectOk(wereAddressesSpentFrom(address));
assertTrue(result, result.startsWith("{\"states\":"));
assertThat(result).startsWith("{\"states\":");
}
@Test
public void givenCheckConsistencyWhenPostThenOk() {
NodeInfo nodeInfo = postCommandExpectOk(command(GET_NODE_INFO.getKey()), NodeInfo.class);
String result = postCommandExpectOk(checkConsistency(nodeInfo.getLatestSolidSubtangleMilestone()));
assertTrue(result, result.startsWith("{\"state\":")); // could be false, too. in case of max depth, ...
assertThat(result).startsWith("{\"state\":"); // could be false, too. in case of max depth, ...
}
@Test
public void givenGetMissingTransactionsWhenPostThenReturnTips() {
String result = postCommandExpectOk(command("getMissingTransactions"));
assertTrue(result, result.startsWith("{\"hashes\":["));
assertThat(result).startsWith("{\"hashes\":[");
}
@Test
public void givenGetNeighborsWhenPostThenReturnNeighbors() {
String result = postCommandExpectOk(getNeighborsCommand());
assertTrue(result, result.startsWith("{\"neighbors\":["));
assertThat(result).startsWith("{\"neighbors\":[");
}
@Test
public void givenAddNeighborsWhenPostThenReturnNumberOfAdded() {
String result = postCommandExpectOk(addNeighborsCommand("udp://8.8.8.8:14600"));
assertTrue(result, result.startsWith("{\"addedNeighbors\":"));
assertThat(result).startsWith("{\"addedNeighbors\":");
util.postCommand(removeNeighborsCommand("udp://8.8.8.8:14600")).expectStatus().isOk(); // clean up
}
@Test
public void givenRemoveNeighborsWhenPostThenReturnNumberOfRemoved() {
String result = postCommandExpectOk(removeNeighborsCommand("udp://8.8.8.8:14600"));
assertTrue(result, result.startsWith("{\"removedNeighbors\":"));
assertThat(result).startsWith("{\"removedNeighbors\":");
}
// Attach to tangle validation errors:
......@@ -276,7 +291,7 @@ public class IriApiHandlerCommandsSysIT {
T result = util.postCommand(command)
.expectStatus().isOk()
.expectBody(responseClass).returnResult().getResponseBody();
assertNotNull(result);
assertThat(result).isNotNull();
logger.debug("result: {}", result.toString());
return result;
}
......
......@@ -22,6 +22,7 @@ package com.mio.piri.service;
import com.mio.piri.commands.GetNodeInfo;
import com.mio.piri.commands.response.NodeInfo;
import com.mio.piri.util.JsonUtil;
import com.mio.piri.util.TestCommandFactory;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.slf4j.Logger;
......@@ -34,7 +35,11 @@ import java.util.HashSet;
import static org.assertj.core.api.Assertions.assertThat;
@RunWith(SpringRunner.class)
@SpringBootTest(properties = {"iri.nodes="}, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@SpringBootTest(properties = {
"iri.nodes=",
"piri.retry.successive-on-same-node=true", // for test with retries
"piri.retry.max-per-node=5" // for test with retries
}, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
public class IriApiHandlerIT extends AbstractIriApiHandlerTest {
private final Logger logger = LoggerFactory.getLogger(getClass());
......@@ -70,4 +75,27 @@ public class IriApiHandlerIT extends AbstractIriApiHandlerTest {
logger.info("Response body: {}", body);
}
@Test
public void givenTipsAreNotConsistentWhenExchangeThenRetry() {
for (int i = 0; i < 3; i++) {
prepareResponse(response -> response
.setHeader("Content-Type", "application/json")
.setResponseCode(400)
.setBody("Tips are not consistent"));
}
prepareResponse(response -> response
.setHeader("Content-Type", "application/json")
.setResponseCode(400)
.setBody("Final response."));
String response = util.postCommand(TestCommandFactory.getBalances())
.expectStatus().isBadRequest()
.expectBody(String.class).returnResult().getResponseBody();
assertThat(response).contains("Final response.");
}
}
\ No newline at end of file
......@@ -315,7 +315,7 @@ public class IriApiHandlerTest {
@Test
public void givenCriticalErrorWhenExchangeThenThrow() {
when(response.getStatusCode()).thenReturn(HttpStatus.INTERNAL_SERVER_ERROR);
when(responseErrorHandler.isCriticalError(any(HttpStatus.class))).thenReturn(true);
when(responseErrorHandler.isCriticalError(any(HttpStatus.class), any())).thenReturn(true);
when(node.call(any(IriCommand.class))).thenReturn(Mono.just(response));
Mono<ResponseEntity<String>> responseMono = iriApiHandler.exchangeWithIri(command, httpRequest);
StepVerifier.create(responseMono)
......@@ -325,7 +325,7 @@ public class IriApiHandlerTest {
@Test
public void givenCriticalErrorWhenExchangeThenRetry() {
when(response.getStatusCode()).thenReturn(HttpStatus.INTERNAL_SERVER_ERROR);
when(responseErrorHandler.isCriticalError(any(HttpStatus.class))).thenReturn(true).thenReturn(false);
when(responseErrorHandler.isCriticalError(any(HttpStatus.class), any())).thenReturn(true).thenReturn(false);
when(node.call(any(IriCommand.class))).thenReturn(Mono.just(response));
Mono<ResponseEntity<String>> responseMono = iriApiHandler.exchangeWithIri(command, httpRequest).log();
StepVerifier.create(responseMono)
......
......@@ -116,8 +116,8 @@ public class TestCommandFactory {
public static GetBalances getBalances(String... address) {
GetBalances gb = new GetBalances();
gb.setCommand("getBalances");
gb.setAddresses(Arrays.asList(address));
gb.setThreshold(100);
gb.setAddresses(Arrays.asList(address));
return gb;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment