Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(restli): log aspect-not-found as a warning rather than as an error #10834

Merged
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.linkedin.metadata.filter;

import com.linkedin.metadata.restli.NonExceptionHttpErrorResponse;
import com.linkedin.restli.common.HttpMethod;
import com.linkedin.restli.common.HttpStatus;
import com.linkedin.restli.server.filter.Filter;
Expand Down Expand Up @@ -33,7 +34,9 @@ public CompletableFuture<Void> onError(
final FilterRequestContext requestContext,
final FilterResponseContext responseContext) {
logResponse(requestContext, responseContext);
log.error("Rest.li error: ", th);
if (!(th instanceof NonExceptionHttpErrorResponse)) {
log.error("Rest.li error: ", th);
}
return CompletableFuture.completedFuture(null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@
import com.codahale.metrics.MetricRegistry;
import com.datahub.authentication.Authentication;
import com.datahub.authentication.AuthenticationContext;
import com.datahub.authorization.EntitySpec;
import com.datahub.plugins.auth.authorization.Authorizer;
import com.google.common.annotations.VisibleForTesting;
import com.linkedin.aspect.GetTimeseriesAspectValuesResponse;
import com.linkedin.common.AuditStamp;
import com.linkedin.common.urn.Urn;
import com.linkedin.metadata.aspect.EnvelopedAspectArray;
import com.linkedin.metadata.aspect.VersionedAspect;
import com.linkedin.metadata.authorization.Disjunctive;
import com.linkedin.metadata.authorization.PoliciesConfig;
import com.linkedin.metadata.entity.EntityService;
import com.linkedin.metadata.entity.IngestResult;
Expand Down Expand Up @@ -153,9 +151,8 @@ public Task<AnyRecord> get(
final VersionedAspect aspect =
_entityService.getVersionedAspect(opContext, urn, aspectName, version);
if (aspect == null) {
throw RestliUtil.resourceNotFoundException(
String.format(
"Did not find urn: %s aspect: %s version: %s", urn, aspectName, version));
log.warn("Did not find urn: {} aspect: {} version: {}", urn, aspectName, version);
throw RestliUtil.nonExceptionResourceNotFound();
}
return new AnyRecord(aspect.data());
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.linkedin.metadata.restli;

import com.linkedin.restli.common.HttpStatus;
import com.linkedin.restli.server.RestLiServiceException;
import com.linkedin.restli.server.errors.ServiceError;

/**
* Captures an error <i>response</i> (e.g. 404-not-found) that is not to be regarded as an
* <i>exception</i> within the server. <br>
* <br>
* Restli apparently requires http-error-responses to be represented by {@link
* RestLiServiceException}; thus, we need this class to specify an error <i>response</i> that isn't
* really an <i>exception</i> (in the context of the server). <br>
* To highlight the unusual purpose of this exception, the name of this class is also deliberately
* unconventional (the class-name doesn't end with "Exception").
*/
public class NonExceptionHttpErrorResponse extends RestLiServiceException {

public NonExceptionHttpErrorResponse(HttpStatus status) {
super(status);
}

public NonExceptionHttpErrorResponse(HttpStatus status, String message) {
super(status, message);
}

public NonExceptionHttpErrorResponse(HttpStatus status, Throwable cause) {
super(status, cause);
}

public NonExceptionHttpErrorResponse(HttpStatus status, String message, Throwable cause) {
super(status, message, cause);
}

public NonExceptionHttpErrorResponse(
HttpStatus status, String message, Throwable cause, boolean writableStackTrace) {
super(status, message, cause, writableStackTrace);
}

public NonExceptionHttpErrorResponse(ServiceError serviceError) {
super(serviceError);
}

public NonExceptionHttpErrorResponse(ServiceError serviceError, Throwable cause) {
super(serviceError, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ public static RestLiServiceException resourceNotFoundException() {
return resourceNotFoundException(null);
}

@Nonnull
public static RestLiServiceException nonExceptionResourceNotFound() {
return new NonExceptionHttpErrorResponse(HttpStatus.S_404_NOT_FOUND);
}

@Nonnull
public static RestLiServiceException resourceNotFoundException(@Nullable String message) {
return new RestLiServiceException(HttpStatus.S_404_NOT_FOUND, message);
Expand Down
Loading