Skip to content

Commit

Permalink
[Bugfix] Better exception handling in search pipelines
Browse files Browse the repository at this point in the history
Thanks to @noCharger for reporting a failing negative test case.

Since we were rethrowing exceptions when resolving search pipelines and
processing search requests, that could end up killing the listener
thread.

Also, we want to make sure that any exception thrown from search
pipelines are wrapped in SearchPipelineProcessingException.

Signed-off-by: Michael Froh <froh@amazon.com>
  • Loading branch information
msfroh committed May 24, 2023
1 parent 49fafd1 commit 566abcf
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,23 @@ teardown:
index: test
body: { }
- match: { hits.total.value: 2 }
---
"Test invalid inline query":
- do:
catch: bad_request
search:
index: test
body: {
search_pipeline: {
"request_processors": [
"filter_query": {
"query": {
"woozlewuzzle": {
"field": "foo"
}
}
}
]
}
}
- match: { status: 400 }
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ private void executeRequest(
);
} catch (Exception e) {
originalListener.onFailure(e);
throw new RuntimeException(e);
return;

Check warning on line 404 in server/src/main/java/org/opensearch/action/search/TransportSearchAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/search/TransportSearchAction.java#L404

Added line #L404 was not covered by tests
}

ActionListener<SearchSourceBuilder> rewriteListener = ActionListener.wrap(source -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,12 @@ public PipelinedRequest resolvePipeline(SearchRequest searchRequest) throws Exce
pipeline = pipelineHolder.pipeline;
}
}
SearchRequest transformedRequest = pipeline.transformRequest(searchRequest);
return new PipelinedRequest(pipeline, transformedRequest);
try {
SearchRequest transformedRequest = pipeline.transformRequest(searchRequest);
return new PipelinedRequest(pipeline, transformedRequest);
} catch (Exception e) {
throw new SearchPipelineProcessingException(e);

Check warning on line 407 in server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java#L406-L407

Added lines #L406 - L407 were not covered by tests
}
}

Map<String, Processor.Factory<SearchRequestProcessor>> getRequestProcessorFactories() {
Expand Down

0 comments on commit 566abcf

Please sign in to comment.