Skip to content

Commit

Permalink
[server] refactor exceptions handling (#1749)
Browse files Browse the repository at this point in the history
  • Loading branch information
cyrilou242 authored Jan 14, 2025
1 parent 541fb96 commit a75697c
Show file tree
Hide file tree
Showing 15 changed files with 156 additions and 216 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.google.inject.Injector;
import io.dropwizard.testing.DropwizardTestSupport;
import jakarta.ws.rs.client.Client;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import org.apache.pinot.testcontainer.PinotContainer.PinotVersion;
import org.slf4j.Logger;
Expand Down Expand Up @@ -76,7 +75,7 @@ public void tearDown() {
}

public DataSourceApi getPinotDataSourceApi()
throws ExecutionException, InterruptedException {
throws Exception {
return pinotDataSourceFuture.get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import ai.startree.thirdeye.plugins.datasource.pinot.resultset.ThirdEyeResultSetGroup;
import ai.startree.thirdeye.spi.Constants;
import ai.startree.thirdeye.spi.ThirdEyeException;
import ai.startree.thirdeye.spi.ThirdEyeStatus;
import ai.startree.thirdeye.spi.api.DemoDatasetApi;
import ai.startree.thirdeye.spi.datalayer.dto.DataSourceDTO;
import ai.startree.thirdeye.spi.datalayer.dto.DatasetConfigDTO;
Expand Down Expand Up @@ -160,11 +161,10 @@ public String getName() {
*
* @param pinotQuery the query that is specifically constructed for Pinot.
* @return the corresponding ResultSetGroup to the given Pinot query.
* @throws ExecutionException is thrown if failed to connect to Pinot or gets results from
* @throws ThirdEyeException is thrown if failed to connect to Pinot or gets results from
* Pinot.
*/
private ThirdEyeResultSetGroup executeSQL(final PinotQuery pinotQuery) throws ExecutionException,
ThirdEyeException {
private ThirdEyeResultSetGroup executeSQL(final PinotQuery pinotQuery) throws ThirdEyeException {
try {
final ThirdEyeResultSetGroup thirdEyeResultSetGroup = queryCache.get(pinotQuery);
final long current = System.currentTimeMillis();
Expand All @@ -175,20 +175,15 @@ private ThirdEyeResultSetGroup executeSQL(final PinotQuery pinotQuery) throws Ex
queryCacheTs = current;
}
return thirdEyeResultSetGroup;
} catch (final ExecutionException e) {
LOG.error("Failed to execute SQL: {} with options {}", pinotQuery.getQuery(),
pinotQuery.getOptions(), e);
LOG.error("queryCache.stats: {}", queryCache.stats());
throw e;
} catch (UncheckedExecutionException e) {
} catch (ExecutionException | UncheckedExecutionException e) {
LOG.error("Failed to execute SQL: {} with options {}", pinotQuery.getQuery(),
pinotQuery.getOptions(), e);
LOG.error("queryCache.stats: {}", queryCache.stats());
Throwable cause = e.getCause();
if (cause instanceof ThirdEyeException) {
throw (ThirdEyeException) cause;
} else {
throw new ExecutionException(e.getMessage(), e);
throw new ThirdEyeException(e, ThirdEyeStatus.ERR_PINOT_QUERY_EXECUTION, e.getMessage(), pinotQuery.getQuery());
}
}
}
Expand All @@ -212,14 +207,14 @@ public DataTable fetchDataTable(final DataSourceRequest request) throws Exceptio
public boolean validate() {
try {
return validate0();
} catch (final ExecutionException | IOException | ArrayIndexOutOfBoundsException | ThirdEyeException e) {
} catch (final IOException | ArrayIndexOutOfBoundsException | ThirdEyeException e) {
LOG.error("Exception while performing pinot datasource validation.", e);
}
return false;
}

// todo cyril healthcheck should be abstracted by the controller
private boolean validate0() throws IOException, ExecutionException, ThirdEyeException {
private boolean validate0() throws IOException, ThirdEyeException {
final PinotHealthCheckConfiguration healthCheck = config.getHealthCheck();
if (healthCheck == null || !healthCheck.isEnabled()) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import ai.startree.thirdeye.config.ThirdEyeServerConfiguration;
import ai.startree.thirdeye.datalayer.DataSourceBuilder;
import ai.startree.thirdeye.detectionpipeline.PlanExecutor;
import ai.startree.thirdeye.exception.GenericExceptionMapper;
import ai.startree.thirdeye.exception.ThirdEyeExceptionMapper;
import ai.startree.thirdeye.exception.ThirdEyeJsonProcessingExceptionMapper;
import ai.startree.thirdeye.healthcheck.DatabaseHealthCheck;
Expand Down Expand Up @@ -152,6 +153,7 @@ public void run(final ThirdEyeServerConfiguration configuration, final Environme
registerResources(env.jersey());
env.jersey().register(new ThirdEyeJsonProcessingExceptionMapper());
env.jersey().register(new ThirdEyeExceptionMapper());
env.jersey().register(new GenericExceptionMapper());

// Persistence layer connectivity health check registry
env.healthChecks().register("database", injector.getInstance(DatabaseHealthCheck.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import static ai.startree.thirdeye.ResourceUtils.ensure;
import static ai.startree.thirdeye.alert.AlertEvaluatorResponseMapper.toAlertEvaluationApi;
import static ai.startree.thirdeye.exception.ExceptionHandler.handleAlertEvaluationException;
import static ai.startree.thirdeye.mapper.ApiBeanMapper.toAlertTemplateApi;
import static ai.startree.thirdeye.spi.util.SpiUtils.bool;
import static ai.startree.thirdeye.spi.util.SpiUtils.optional;
Expand All @@ -35,10 +34,8 @@
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import jakarta.ws.rs.WebApplicationException;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -82,20 +79,7 @@ private void stop() {

// does not resolve namespace - assumes namespace is set in the request by the consumer
@VisibleForTesting
public AlertEvaluationApi evaluate(final AlertEvaluationApi request)
throws ExecutionException {
try {
return evaluate0(request);
} catch (final WebApplicationException e) {
throw e;
} catch (final Exception e) {
handleAlertEvaluationException(e);
}
return null;
}

private AlertEvaluationApi evaluate0(final AlertEvaluationApi request)
throws Exception {
public AlertEvaluationApi evaluate(final AlertEvaluationApi request) throws Exception {
final long startTime = request.getStart().getTime();
final long endTime = request.getEnd().getTime();
final String namespace = optional(request.getAlert().getAuth()).map(
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright 2024 StarTree Inc
*
* Licensed under the StarTree Community License (the "License"); you may not use
* this file except in compliance with the License. You may obtain a copy of the
* License at http://www.startree.ai/legal/startree-community-license
*
* Unless required by applicable law or agreed to in writing, software distributed under the
* License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OF ANY KIND,
* either express or implied.
* See the License for the specific language governing permissions and limitations under
* the License.
*/
package ai.startree.thirdeye.exception;

import ai.startree.thirdeye.spi.api.ExceptionApi;
import ai.startree.thirdeye.spi.api.StackTraceElementApi;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

public class ExceptionUtils {

public static ExceptionApi toExceptionApi(final Throwable t) {
if (t == null) {
return null;
}

final List<StackTraceElementApi> stackTrace = Arrays.stream(t.getStackTrace())
.map(ExceptionUtils::stackTraceElementApi)
.collect(Collectors.toList());

return new ExceptionApi()
.setMessage(t.getMessage())
.setCause(toExceptionApi(t.getCause()))
.setStackTrace(stackTrace);
}

private static StackTraceElementApi stackTraceElementApi(final StackTraceElement ste) {
return new StackTraceElementApi()
.setClassName(ste.getClassName())
.setMethodName(ste.getMethodName())
.setFileName(ste.getFileName())
.setLineNumber(ste.getLineNumber());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright 2024 StarTree Inc
*
* Licensed under the StarTree Community License (the "License"); you may not use
* this file except in compliance with the License. You may obtain a copy of the
* License at http://www.startree.ai/legal/startree-community-license
*
* Unless required by applicable law or agreed to in writing, software distributed under the
* License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OF ANY KIND,
* either express or implied.
* See the License for the specific language governing permissions and limitations under
* the License.
*/
package ai.startree.thirdeye.exception;

import static ai.startree.thirdeye.exception.ExceptionUtils.toExceptionApi;

import ai.startree.thirdeye.spi.ThirdEyeStatus;
import ai.startree.thirdeye.spi.api.StatusApi;
import ai.startree.thirdeye.spi.api.StatusListApi;
import io.dropwizard.jersey.errors.LoggingExceptionMapper;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.Provider;
import java.util.List;
import java.util.concurrent.TimeoutException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This mapper has lesser priority than all others.
* Most Exceptions thrown implemented in this code base should be wrapped inside a ThirdEyeException
* and will benefit from the ThirdEyeExceptionMapper
* Complete this class only for exceptions that are hard to wrap with a ThirdEyeException.
*/
@Provider
public class GenericExceptionMapper extends LoggingExceptionMapper<Throwable> {

private static final Logger LOG = LoggerFactory.getLogger(GenericExceptionMapper.class);

public GenericExceptionMapper() {
}

@Override
public Response toResponse(final Throwable exception) {
final ThirdEyeStatus status = statusFor(exception);
LOG.debug(
"Request failed because of a {}. Returning error code {}",
exception.getClass().getSimpleName(),
status.getRecommendedStatusCode());
final StatusApi statusApi = new StatusApi()
.setCode(status)
.setMsg(exception.getMessage())
// TODO cyril put this behind a boolean - in some environments we should not return this
.setException(toExceptionApi(exception));
final StatusListApi statusList = new StatusListApi().setList(List.of(statusApi));
return Response.status(status.getRecommendedStatusCode())
.type(MediaType.APPLICATION_JSON_TYPE)
.entity(statusList)
.build();
}

private static ThirdEyeStatus statusFor(final Throwable exception) {
if (exception instanceof TimeoutException) {
return ThirdEyeStatus.ERR_TIMEOUT;
} else {
return ThirdEyeStatus.ERR_UNKNOWN;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
package ai.startree.thirdeye.exception;

import static ai.startree.thirdeye.exception.ExceptionHandler.toExceptionApi;
import static ai.startree.thirdeye.exception.ExceptionUtils.toExceptionApi;

import ai.startree.thirdeye.spi.ThirdEyeException;
import ai.startree.thirdeye.spi.api.StatusApi;
Expand Down
Loading

0 comments on commit a75697c

Please sign in to comment.