Skip to content

Commit

Permalink
fix(restli): log aspect-not-found as a warning rather than as an error (
Browse files Browse the repository at this point in the history
  • Loading branch information
ksrinath authored Jul 4, 2024
1 parent fa3e381 commit 906bc98
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 6 deletions.
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

0 comments on commit 906bc98

Please sign in to comment.