Skip to content

Commit

Permalink
Returns parse exceptions as 400 (#122)
Browse files Browse the repository at this point in the history
Parse exceptions should be returned as 400s

<img width="1572" alt="image"
src="https://github.com/QuesmaOrg/quesma/assets/2182533/ea8a3123-bbca-4c32-a292-594276828b48">
  • Loading branch information
pivovarit authored May 16, 2024
1 parent 6adefb8 commit 7cc7415
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 17 deletions.
12 changes: 6 additions & 6 deletions quesma/eql/query_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (cw *ClickhouseEQLQueryTranslator) MakeSearchResponse(ResultSet []model.Que
}, nil
}

func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query queryparser.SimpleQuery, searchQueryInfo model.SearchQueryInfo, highlighter model.Highlighter) {
func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query queryparser.SimpleQuery, searchQueryInfo model.SearchQueryInfo, highlighter model.Highlighter, err error) {

// no highlighting here
highlighter = queryparser.NewEmptyHighlighter()
Expand All @@ -97,13 +97,13 @@ func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query qu
query.Sql = queryparser.Statement{}

queryAsMap := make(map[string]interface{})
err := json.Unmarshal([]byte(queryAsJson), &queryAsMap)
err = json.Unmarshal([]byte(queryAsJson), &queryAsMap)
if err != nil {
logger.ErrorWithCtx(cw.Ctx).Err(err).Msg("error parsing query request's JSON")

query.CanParse = false
query.Sql.Stmt = "Invalid JSON"
return query, model.NewSearchQueryInfoNone(), highlighter
return query, model.NewSearchQueryInfoNone(), highlighter, err
}

var eqlQuery string
Expand All @@ -115,7 +115,7 @@ func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query qu
if eqlQuery == "" {
query.CanParse = false
query.Sql.Stmt = "Empty query"
return query, model.NewSearchQueryInfoNone(), highlighter
return query, model.NewSearchQueryInfoNone(), highlighter, nil
}

// FIXME this is a naive translation.
Expand All @@ -138,14 +138,14 @@ func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query qu
logger.ErrorWithCtx(cw.Ctx).Err(err).Msgf("error transforming EQL query: '%s'", eqlQuery)
query.CanParse = false
query.Sql.Stmt = "Invalid EQL query"
return query, model.NewSearchQueryInfoNone(), highlighter
return query, model.NewSearchQueryInfoNone(), highlighter, err
}

query.Sql.Stmt = where
query.CanParse = true
query.SortFields = []string{"\"@timestamp\""}

return query, searchQueryInfo, highlighter
return query, searchQueryInfo, highlighter, nil
}

// These methods are not supported by EQL. They are here to satisfy the interface.
Expand Down
7 changes: 3 additions & 4 deletions quesma/queryparser/query_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,14 @@ func NewCompoundStatementNoFieldName(stmt string) Statement {
return Statement{Stmt: stmt, isCompound: true}
}

func (cw *ClickhouseQueryTranslator) ParseQuery(queryAsJson string) (SimpleQuery, model.SearchQueryInfo, model.Highlighter) {
func (cw *ClickhouseQueryTranslator) ParseQuery(queryAsJson string) (SimpleQuery, model.SearchQueryInfo, model.Highlighter, error) {
cw.ClearTokensToHighlight()
queryAsMap := make(QueryMap)
if queryAsJson != "" {
err := json.Unmarshal([]byte(queryAsJson), &queryAsMap)
if err != nil {
logger.ErrorWithCtx(cw.Ctx).Err(err).Msg("error parsing query request's JSON")
return newSimpleQuery(NewSimpleStatement("invalid JSON (ParseQuery)"), false),
model.NewSearchQueryInfoNone(), NewEmptyHighlighter()
return SimpleQuery{}, model.SearchQueryInfo{}, NewEmptyHighlighter(), err
}
}

Expand Down Expand Up @@ -106,7 +105,7 @@ func (cw *ClickhouseQueryTranslator) ParseQuery(queryAsJson string) (SimpleQuery
highlighter.SetTokens(cw.tokensToHighlight)
cw.ClearTokensToHighlight()

return parsedQuery, queryInfo, highlighter
return parsedQuery, queryInfo, highlighter, nil
}

func (cw *ClickhouseQueryTranslator) ParseHighlighter(queryMap QueryMap) model.Highlighter {
Expand Down
6 changes: 3 additions & 3 deletions quesma/queryparser/query_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestQueryParserStringAttrConfig(t *testing.T) {

for _, tt := range testdata.TestsSearch {
t.Run(tt.Name, func(t *testing.T) {
simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson)
simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson)
assert.True(t, simpleQuery.CanParse, "can parse")
assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql")
assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type")
Expand All @@ -74,7 +74,7 @@ func TestQueryParserNoFullTextFields(t *testing.T) {

for i, tt := range testdata.TestsSearchNoFullTextFields {
t.Run(strconv.Itoa(i), func(t *testing.T) {
simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson)
simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson)
assert.True(t, simpleQuery.CanParse, "can parse")
assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql")
assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type")
Expand All @@ -99,7 +99,7 @@ func TestQueryParserNoAttrsConfig(t *testing.T) {
cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: table, Ctx: context.Background()}
for _, tt := range testdata.TestsSearchNoAttrs {
t.Run(tt.Name, func(t *testing.T) {
simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson)
simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson)
assert.True(t, simpleQuery.CanParse)
assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt)
assert.Equal(t, tt.WantedQueryType, queryInfo.Typ)
Expand Down
39 changes: 39 additions & 0 deletions quesma/queryparser/query_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package queryparser

import (
"context"
"encoding/json"
"fmt"
"mitmproxy/quesma/clickhouse"
"mitmproxy/quesma/kibana"
Expand Down Expand Up @@ -133,6 +134,44 @@ func EmptyAsyncSearchResponse(id string, isPartial bool, completionStatus int) (
return asyncSearchResp.Marshal() // error should never ever happen here
}

func BadRequestParseError(err error) []byte {
serialized, _ := json.Marshal(ParseErrorResponse{
Error: Error{
RootCause: []RootCause{
{
Type: "parsing_exception",
Reason: err.Error(),
},
},
Type: "parsing_exception",
Reason: err.Error(),
},
Status: 400,
},
)
return serialized
}

type (
ParseErrorResponse struct {
Error `json:"error"`
Status int `json:"status"`
}
Error struct {
RootCause []RootCause `json:"root_cause"`
Type string `json:"type"`
Reason string `json:"reason"`
Line *int `json:"line,omitempty"`
Col *int `json:"col,omitempty"`
}
RootCause struct {
Type string `json:"type"`
Reason string `json:"reason"`
Line *int `json:"line,omitempty"`
Col *int `json:"col,omitempty"`
}
)

func (cw *ClickhouseQueryTranslator) MakeSearchResponse(ResultSet []model.QueryResultRow, typ model.SearchQueryType, highlighter model.Highlighter) (*model.SearchResp, error) {
switch typ {
case model.Normal:
Expand Down
2 changes: 1 addition & 1 deletion quesma/quesma/query_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
// 2. ClickhouseEQLQueryTranslator (implements only a subset of methods)

type IQueryTranslator interface {
ParseQuery(queryAsJson string) (queryparser.SimpleQuery, model.SearchQueryInfo, model.Highlighter)
ParseQuery(queryAsJson string) (queryparser.SimpleQuery, model.SearchQueryInfo, model.Highlighter, error)
ParseAggregationJson(aggregationJson string) ([]model.QueryWithAggregation, error)

BuildSimpleCountQuery(whereClause string) *model.Query
Expand Down
11 changes: 11 additions & 0 deletions quesma/quesma/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"mitmproxy/quesma/clickhouse"
"mitmproxy/quesma/elasticsearch"
"mitmproxy/quesma/logger"
"mitmproxy/quesma/queryparser"
"mitmproxy/quesma/quesma/config"
"mitmproxy/quesma/quesma/mux"
"mitmproxy/quesma/quesma/routes"
Expand Down Expand Up @@ -158,6 +159,11 @@ func configureRouter(cfg config.QuesmaConfiguration, lm *clickhouse.LogManager,
if err != nil {
if errors.Is(errIndexNotExists, err) {
return &mux.Result{StatusCode: 404}, nil
} else if errors.Is(err, errCouldNotParseRequest) {
return &mux.Result{
Body: string(queryparser.BadRequestParseError(err)),
StatusCode: 400,
}, nil
} else {
return nil, err
}
Expand All @@ -183,6 +189,11 @@ func configureRouter(cfg config.QuesmaConfiguration, lm *clickhouse.LogManager,
if err != nil {
if errors.Is(errIndexNotExists, err) {
return &mux.Result{StatusCode: 404}, nil
} else if errors.Is(err, errCouldNotParseRequest) {
return &mux.Result{
Body: string(queryparser.BadRequestParseError(err)),
StatusCode: 400,
}, nil
} else {
return nil, err
}
Expand Down
11 changes: 9 additions & 2 deletions quesma/quesma/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ import (
const asyncQueriesLimit = 10000
const asyncQueriesLimitBytes = 1024 * 1024 * 500 // 500MB

var errIndexNotExists = errors.New("table does not exist")
var (
errIndexNotExists = errors.New("table does not exist")
errCouldNotParseRequest = errors.New("parse exception")
)
var asyncRequestId atomic.Int64

type AsyncRequestResult struct {
Expand Down Expand Up @@ -207,7 +210,11 @@ func (q *QueryRunner) handleSearchCommon(ctx context.Context, indexPattern strin

queryTranslator = NewQueryTranslator(ctx, queryLanguage, table, q.logManager)

simpleQuery, queryInfo, highlighter = queryTranslator.ParseQuery(string(body))
simpleQuery, queryInfo, highlighter, err = queryTranslator.ParseQuery(string(body))
if err != nil {
logger.ErrorWithCtx(ctx).Msgf("error parsing query: %v", err)
return nil, errors.Join(errCouldNotParseRequest, err)
}

if simpleQuery.CanParse {
if isNonAggregationQuery(queryInfo, body) {
Expand Down
2 changes: 1 addition & 1 deletion quesma/quesma/search_opensearch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestSearchOpensearch(t *testing.T) {
managementConsole := ui.NewQuesmaManagementConsole(cfg, nil, nil, make(<-chan tracing.LogWithLevel, 50000), telemetry.NewPhoneHomeEmptyAgent())
cw := queryparser.ClickhouseQueryTranslator{ClickhouseLM: lm, Table: &table, Ctx: context.Background()}

simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson)
simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson)
assert.True(t, simpleQuery.CanParse, "can parse")
assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql")
assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type")
Expand Down

0 comments on commit 7cc7415

Please sign in to comment.