Skip to content

Commit

Permalink
Panic HTTP error handling (#136)
Browse files Browse the repository at this point in the history
When we got panic, we responded with HTTP 200 but no right headers; this
needs to be clarified.

Fixed:
- use HTTP 500
- Quesma should take responsibility and return JSON

Before:
![Screenshot 2024-05-17 at 11 46
47](https://github.com/QuesmaOrg/quesma/assets/972989/04395b1f-b0b5-4a42-9b8c-9ea6aed5d7aa)

After:
![Screenshot 2024-05-17 at 12 16
57](https://github.com/QuesmaOrg/quesma/assets/972989/8120e844-e7f7-4b6c-bc0b-509db54ec773)
  • Loading branch information
jakozaur authored May 17, 2024
1 parent b04ee53 commit a7a03b9
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 40 deletions.
59 changes: 59 additions & 0 deletions quesma/queryparser/dashboard_error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package queryparser

import "encoding/json"

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

func InternalQuesmaError(msg string) []byte {
serialized, _ := json.Marshal(DashboardErrorResponse{
Error: Error{
RootCause: []RootCause{
{
Type: "quesma_error",
Reason: msg,
},
},
Type: "quesma_error",
Reason: msg,
},
Status: 500,
},
)
return serialized
}

type (
DashboardErrorResponse 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"`
}
)
39 changes: 0 additions & 39 deletions quesma/queryparser/query_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package queryparser

import (
"context"
"encoding/json"
"fmt"
"mitmproxy/quesma/clickhouse"
"mitmproxy/quesma/kibana"
Expand Down Expand Up @@ -134,44 +133,6 @@ 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
6 changes: 5 additions & 1 deletion quesma/quesma/quesma.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"mitmproxy/quesma/logger"
"mitmproxy/quesma/network"
"mitmproxy/quesma/proxy"
"mitmproxy/quesma/queryparser"
"mitmproxy/quesma/quesma/config"
"mitmproxy/quesma/quesma/gzip"
"mitmproxy/quesma/quesma/mux"
Expand Down Expand Up @@ -116,7 +117,10 @@ type router struct {
}

func (r *router) reroute(ctx context.Context, w http.ResponseWriter, req *http.Request, reqBody []byte, router *mux.PathRouter, logManager *clickhouse.LogManager) {
defer recovery.LogPanicWithCtx(ctx)
defer recovery.LogAndHandlePanic(ctx, func() {
w.WriteHeader(500)
w.Write(queryparser.InternalQuesmaError("Unknown Quesma error"))
})
if router.Matches(req.URL.Path, req.Method, string(reqBody)) {
var elkResponseChan = make(chan elasticResult)

Expand Down
9 changes: 9 additions & 0 deletions quesma/quesma/recovery/recovery_strategies.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,12 @@ func LogPanicWithCtx(ctx context.Context) {
})
}
}

func LogAndHandlePanic(ctx context.Context, cleanupHandler func()) {
if r := recover(); r != nil {
commonRecovery(r, func() *zerolog.Event {
return logger.ErrorWithCtx(ctx)
})
cleanupHandler()
}
}

0 comments on commit a7a03b9

Please sign in to comment.