Skip to content

Commit

Permalink
Remove HttpClient logging stacktrace (#743)
Browse files Browse the repository at this point in the history
  • Loading branch information
rpanchyk authored Jun 2, 2020
1 parent 455e81c commit fa0429c
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ private Future<Account> accountFallback(Throwable exception, String accountId,
private Future<Account> responseForUnknownAccount(String accountId) {
return enforceValidAccount
? Future.failedFuture(new UnauthorizedAccountException(
String.format("Unauthorised account id %s", accountId), accountId))
String.format("Unauthorized account id: %s", accountId), accountId))
: Future.succeededFuture(Account.empty(accountId));
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package org.prebid.server.exception;

import lombok.Getter;

import java.util.Collections;
import java.util.List;

@SuppressWarnings("serial")
public class InvalidRequestException extends RuntimeException {

@Getter
private final List<String> messages;

@Getter
private final boolean needEnhancedLogging;

public InvalidRequestException(String message) {
Expand Down Expand Up @@ -39,12 +43,4 @@ public InvalidRequestException(List<String> messages, boolean needEnhancedLoggin
this.messages = messages;
this.needEnhancedLogging = needEnhancedLogging;
}

public List<String> getMessages() {
return messages;
}

public boolean isNeedEnhancedLogging() {
return needEnhancedLogging;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

import lombok.Getter;

@Getter
@SuppressWarnings("serial")
public class UnauthorizedAccountException extends RuntimeException {

@Getter
private String accountId;

public UnauthorizedAccountException(String message, String accountId) {
super(String.format("%s", message));
super(message);
this.accountId = accountId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -319,15 +319,15 @@ private void handleResult(AsyncResult<AmpResponse> responseResult, AmpEvent.AmpE
body = message;
} else if (exception instanceof UnauthorizedAccountException) {
metricRequestStatus = MetricName.badinput;
final String message = String.format("Unauthorized: %s", exception.getMessage());
final String message = exception.getMessage();
conditionalLogger.info(message, 100);

errorMessages = Collections.singletonList(message);

status = HttpResponseStatus.UNAUTHORIZED.code();
body = message;
String userId = ((UnauthorizedAccountException) exception).getAccountId();
metrics.updateAccountRequestRejectedMetrics(userId);
String accountId = ((UnauthorizedAccountException) exception).getAccountId();
metrics.updateAccountRequestRejectedMetrics(accountId);
} else if (exception instanceof BlacklistedAppException
|| exception instanceof BlacklistedAccountException) {
metricRequestStatus = exception instanceof BlacklistedAccountException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private void handleResult(AsyncResult<Tuple2<BidResponse, AuctionContext>> respo
body = message;
} else if (exception instanceof UnauthorizedAccountException) {
metricRequestStatus = MetricName.badinput;
final String message = String.format("Unauthorized: %s", exception.getMessage());
final String message = exception.getMessage();
conditionalLogger.info(message, 100);
errorMessages = Collections.singletonList(message);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
public class GdprService {

private static final Logger logger = LoggerFactory.getLogger(GdprService.class);

private static final int PURPOSE_ONE_ID = 1;

private final VendorListService<VendorListV1, VendorV1> vendorListService;
Expand Down Expand Up @@ -72,7 +73,7 @@ private VendorConsent vendorConsentFrom(String vendorConsentString) {
try {
return VendorConsentDecoder.fromBase64String(vendorConsentString);
} catch (IllegalArgumentException | IllegalStateException e) {
logger.warn("Parsing consent string failed with error: {0}", e.getMessage());
logger.info("Parsing consent string failed with error: {0}", e.getMessage());
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ private GdprInfoWithCountry<String> createGdprInfoWithCountry(String gdprConsent
}

private GdprInfoWithCountry<String> updateMetricsAndReturnDefault(Throwable exception, String gdprConsent) {
logger.info("Geolocation lookup failed", exception);
final String message = String.format("Geolocation lookup failed: %s", exception.getMessage());
logger.warn(message);
logger.debug(message, exception);

metrics.updateGeoLocationMetric(false);
return defaultGdprInfoWithCountry(gdprConsent);
}
Expand Down Expand Up @@ -325,7 +328,7 @@ private static TCString decodeTcString(String consentString) {
try {
return StringUtils.isBlank(consentString) ? null : TCString.decode(consentString);
} catch (Throwable e) {
logger.warn("Parsing consent string failed with error: {0}", e.getMessage());
logger.info("Parsing consent string failed with error: {0}", e.getMessage());
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ private Future<Void> saveToFile(String content, int version) {
logger.info("Created new vendor list for version {0}, file: {1}", version, filepath);
promise.complete();
} else {
logger.warn("Could not create new vendor list for version {0}, file: {1}", result.cause(), version,
logger.error("Could not create new vendor list for version {0}, file: {1}", result.cause(), version,
filepath);
promise.fail(result.cause());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpClientRequest;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.logging.Logger;
import io.vertx.core.logging.LoggerFactory;
import org.prebid.server.vertx.http.model.HttpClientResponse;

import java.util.Objects;
Expand All @@ -18,8 +16,6 @@
*/
public class BasicHttpClient implements HttpClient {

private static final Logger logger = LoggerFactory.getLogger(BasicHttpClient.class);

private final Vertx vertx;
private final io.vertx.core.http.HttpClient httpClient;

Expand Down Expand Up @@ -95,8 +91,6 @@ private void failResponse(Throwable exception, Promise<HttpClientResponse> promi
}

private static void failResponse(Throwable exception, Promise<HttpClientResponse> promise) {
logger.warn("HTTP client error", exception);

promise.tryFail(exception);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public void shouldReturnFailedFutureIfAccountIsEnforcedAndIdIsNotProvided() {
assertThat(future.failed()).isTrue();
assertThat(future.cause())
.isInstanceOf(UnauthorizedAccountException.class)
.hasMessage("Unauthorised account id ");
.hasMessage("Unauthorized account id: ");
}

@Test
Expand Down Expand Up @@ -232,7 +232,7 @@ public void shouldReturnFailedFutureIfAccountIsEnforcedAndFailedGetAccountById()
assertThat(future.failed()).isTrue();
assertThat(future.cause())
.isInstanceOf(UnauthorizedAccountException.class)
.hasMessage("Unauthorised account id absentId");
.hasMessage("Unauthorized account id: absentId");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public void shouldRespondWithBadRequestIfRequestHasBlacklistedApp() {
public void shouldRespondWithUnauthorizedIfAccountIdIsInvalid() {
// given
given(ampRequestFactory.fromRequest(any(), anyLong()))
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided 1", "1")));
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided", null)));

// when
ampHandler.handle(routingContext);
Expand All @@ -272,7 +272,7 @@ public void shouldRespondWithUnauthorizedIfAccountIdIsInvalid() {
.containsOnly(
tuple("AMP-Access-Control-Allow-Source-Origin", "http://example.com"),
tuple("Access-Control-Expose-Headers", "AMP-Access-Control-Allow-Source-Origin"));
verify(httpResponse).end(eq("Unauthorized: Account id is not provided 1"));
verify(httpResponse).end(eq("Account id is not provided"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,15 @@ public void shouldRespondWithBadRequestIfBidRequestIsInvalid() {
public void shouldRespondWithUnauthorizedIfAccountIdIsInvalid() {
// given
given(auctionRequestFactory.fromRequest(any(), anyLong()))
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided 1", "1")));
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided", null)));

// when
auctionHandler.handle(routingContext);

// then
verifyZeroInteractions(exchangeService);
verify(httpResponse).setStatusCode(eq(401));
verify(httpResponse).end(eq("Unauthorized: Account id is not provided 1"));
verify(httpResponse).end(eq("Account id is not provided"));
}

@Test
Expand Down

0 comments on commit fa0429c

Please sign in to comment.