Skip to content

Commit

Permalink
more test fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Nicholas Walter Knize <[email protected]>
  • Loading branch information
nknize committed Jun 7, 2023
1 parent b189c18 commit 4931950
Show file tree
Hide file tree
Showing 14 changed files with 218 additions and 208 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package org.opensearch.client;

import org.opensearch.BaseOpenSearchException;
import org.opensearch.OpenSearchException;
import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.explain.ExplainRequest;
Expand Down Expand Up @@ -828,8 +829,8 @@ public void testSearchScroll() throws Exception {
() -> execute(scrollRequest, highLevelClient()::scroll, highLevelClient()::scrollAsync)
);
assertEquals(RestStatus.NOT_FOUND, exception.status());
assertThat(exception.getRootCause(), instanceOf(OpenSearchException.class));
OpenSearchException rootCause = (OpenSearchException) exception.getRootCause();
assertThat(exception.getRootCause(), instanceOf(BaseOpenSearchException.class));
BaseOpenSearchException rootCause = (BaseOpenSearchException) exception.getRootCause();
assertThat(rootCause.getMessage(), containsString("No search context found for"));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
package org.opensearch.client.documentation;

import org.apache.hc.core5.http.HttpHost;
import org.opensearch.BaseOpenSearchException;
import org.opensearch.OpenSearchException;
import org.opensearch.action.ActionListener;
import org.opensearch.action.DocWriteRequest;
Expand Down Expand Up @@ -1943,7 +1944,7 @@ public void testMultiGet() throws Exception {
// tag::multi-get-indexnotfound
assertNull(missingIndexItem.getResponse()); // <1>
Exception e = missingIndexItem.getFailure().getFailure(); // <2>
OpenSearchException ee = (OpenSearchException) e; // <3>
BaseOpenSearchException ee = (BaseOpenSearchException) e; // <3>
// TODO status is broken! fix in a followup
// assertEquals(RestStatus.NOT_FOUND, ee.status()); // <4>
assertThat(e.getMessage(),
Expand Down Expand Up @@ -2035,7 +2036,7 @@ public void onFailure(Exception e) {
MultiGetItemResponse item = response.getResponses()[0];
assertNull(item.getResponse()); // <1>
Exception e = item.getFailure().getFailure(); // <2>
OpenSearchException ee = (OpenSearchException) e; // <3>
BaseOpenSearchException ee = (BaseOpenSearchException) e; // <3>
// TODO status is broken! fix in a followup
// assertEquals(RestStatus.CONFLICT, ee.status()); // <4>
assertThat(e.getMessage(),
Expand Down
21 changes: 17 additions & 4 deletions libs/core/src/main/java/org/opensearch/BaseExceptionsHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.core.ParseField;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.compress.NotXContentException;
import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException;
import org.opensearch.core.rest.RestStatus;
Expand Down Expand Up @@ -109,7 +110,7 @@ public static String detailedMessage(Throwable t) {
if (t.getCause() != null) {
StringBuilder sb = new StringBuilder();
while (t != null) {
sb.append(t.getClass().getSimpleName());
sb.append(getExceptionSimpleClassName(t));
if (t.getMessage() != null) {
sb.append("[");
sb.append(t.getMessage());
Expand All @@ -123,7 +124,7 @@ public static String detailedMessage(Throwable t) {
}
return sb.toString();
} else {
return t.getClass().getSimpleName() + "[" + t.getMessage() + "]";
return getExceptionSimpleClassName(t) + "[" + t.getMessage() + "]";
}
}

Expand All @@ -137,7 +138,7 @@ public static String stackTrace(Throwable e) {
public static String summaryMessage(Throwable t) {
if (t != null) {
if (t instanceof BaseOpenSearchException) {
return t.getClass().getSimpleName() + "[" + t.getMessage() + "]";
return getExceptionSimpleClassName(t) + "[" + t.getMessage() + "]";
} else if (t instanceof IllegalArgumentException) {
return "Invalid argument";
} else if (t instanceof JsonParseException) {
Expand Down Expand Up @@ -226,14 +227,26 @@ public static void generateThrowableXContent(XContentBuilder builder, ToXContent
* Returns an underscore case name for the given exception. This method strips {@code OpenSearch} prefixes from exception names.
*/
public static String getExceptionName(Throwable ex) {
String simpleName = ex.getClass().getSimpleName();
String simpleName = getExceptionSimpleClassName(ex);
if (simpleName.startsWith("OpenSearch")) {
simpleName = simpleName.substring("OpenSearch".length());
}
// TODO: do we really need to make the exception name in underscore casing?
return toUnderscoreCase(simpleName);
}

private static String getExceptionSimpleClassName(final Throwable ex) {
String simpleName = ex.getClass().getSimpleName();
if (Strings.isEmpty(simpleName) && ex instanceof BaseOpenSearchException) {
simpleName = "OpenSearchException";
}

if (simpleName.startsWith("BaseOpenSearch")) {
simpleName = simpleName.substring("Base".length());
}
return simpleName;
}

// lower cases and adds underscores to transitions in a name
private static String toUnderscoreCase(String value) {
StringBuilder sb = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.opensearch.common.CheckedFunction;
import org.opensearch.common.Nullable;
import org.opensearch.common.collect.Tuple;
import org.opensearch.core.common.ParsingException;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -72,7 +73,7 @@
* @opensearch.internal
*/
@SuppressWarnings("rawtypes")
public abstract class BaseOpenSearchException extends RuntimeException implements Writeable, ToXContentFragment {
public class BaseOpenSearchException extends RuntimeException implements Writeable, ToXContentFragment {

protected static final String ERROR = "error";
protected static final String ROOT_CAUSE = "root_cause";
Expand All @@ -94,6 +95,9 @@ public abstract class BaseOpenSearchException extends RuntimeException implement
UNKNOWN_VERSION_ADDED
)
);
registerExceptionHandle(
new BaseOpenSearchExceptionHandle(ParsingException.class, ParsingException::new, 40, UNKNOWN_VERSION_ADDED)
);
}

/**
Expand Down Expand Up @@ -557,7 +561,11 @@ public void setShard(ShardId shardId) {
}

/**
* An ExceptionHandle for registering Exceptions that can be serialized over the transport wire
* This is the list of Exceptions OpenSearch can throw over the wire or save into a corruption marker. Each value in the enum is a
* single exception tying the Class to an id for use of the encode side and the id back to a constructor for use on the decode side. As
* such its ok if the exceptions to change names so long as their constructor can still read the exception. Each exception is listed
* in id order. If you want to remove an exception leave a tombstone comment and mark the id as null in
* ExceptionSerializationTests.testIds.ids.
*
* @opensearch.internal
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.lucene.search.TotalHits;
import org.opensearch.BaseOpenSearchException;
import org.opensearch.OpenSearchException;
import org.opensearch.Version;
import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsAction;
Expand Down Expand Up @@ -297,19 +298,19 @@ private static void assertSearchConnectFailure() {
{
OpenSearchException exception = expectThrows(OpenSearchException.class,
() -> restHighLevelClient.search(new SearchRequest("index", "remote1:index"), RequestOptions.DEFAULT));
OpenSearchException rootCause = (OpenSearchException)exception.getRootCause();
BaseOpenSearchException rootCause = (BaseOpenSearchException)exception.getRootCause();
assertThat(rootCause.getMessage(), containsString("connect_exception"));
}
{
OpenSearchException exception = expectThrows(OpenSearchException.class,
() -> restHighLevelClient.search(new SearchRequest("remote1:index"), RequestOptions.DEFAULT));
OpenSearchException rootCause = (OpenSearchException)exception.getRootCause();
BaseOpenSearchException rootCause = (BaseOpenSearchException)exception.getRootCause();
assertThat(rootCause.getMessage(), containsString("connect_exception"));
}
{
OpenSearchException exception = expectThrows(OpenSearchException.class,
() -> restHighLevelClient.search(new SearchRequest("remote1:index").scroll("1m"), RequestOptions.DEFAULT));
OpenSearchException rootCause = (OpenSearchException)exception.getRootCause();
BaseOpenSearchException rootCause = (BaseOpenSearchException)exception.getRootCause();
assertThat(rootCause.getMessage(), containsString("connect_exception"));
}
}
Expand Down
Loading

0 comments on commit 4931950

Please sign in to comment.