Add errorsource to errors #449
Conversation
| // the Stats query can be created later. | ||
| func handleServiceMapPrefetch(ctx context.Context, osClient client.Client, req *backend.QueryDataRequest) (string, error) { | ||
| // the Stats query can be created later. Returns true if the serviceMap was fetched | ||
| func handleServiceMapPrefetch(ctx context.Context, osClient client.Client, req *backend.QueryDataRequest) *backend.QueryDataResponse { |
There was a problem hiding this comment.
There wasn't really a better way to handle the fact that execute is now returning the error as part of the response and still wrap the other errors
| } | ||
|
|
||
| func wrapError(response *backend.QueryDataResponse, err error) (*backend.QueryDataResponse, error) { | ||
| var invalidQueryTypeError invalidQueryTypeError |
There was a problem hiding this comment.
now the error gets wrapped earlier, so we don't need this
iwysiu
force-pushed
the
iwysiu/errorsourcewq
branch
from
ad96a22
to
af59d41
Compare
iwysiu
requested review from
kevinwcyu,
nmarrs and
njvrzm
and removed request for
a team
pkg/opensearch/lucene_handler.go
Outdated
| } | ||
|
|
||
| res, err := h.client.ExecuteMultisearch(ctx, req) | ||
| if err != nil { | ||
| return nil, err | ||
| // We are returning the error containing the source that was added through errorsource.Middleware | ||
| return errorsource.AddErrorToResponse(h.queries[0].RefID, response, err), nil |
There was a problem hiding this comment.
Should we maybe draw out the h.queries[0].RefID into a defined reusable var in this case?
| } | ||
|
|
||
| query := newQueryRequest(osClient, req.Queries, req.PluginContext.DataSourceInstanceSettings) | ||
| response, err := wrapError(query.execute(ctx)) | ||
| response, err = wrapError(query.execute(ctx)) |
There was a problem hiding this comment.
what is context of removing the := to =? (I am a bit of a Go noob btw)
There was a problem hiding this comment.
also should we keep this version of wrapError around vs using the newer wrap for this error? or in this case are we confident that this is an error owned by us
There was a problem hiding this comment.
The difference is that := initializes and sets a variable and = sets an existing variable. Since response and err have now both been already initialized in this function, we only need to set it (and the compiler will yell at us if we do otherwise). It was able to use := before because while err had been initialized, response hadn't.
This is the same wrapError as it was before (line 128), it just used to also handle invalidQueryTypeError which we're now handling earlier.
Co-authored-by: Nathan Vērzemnieks <njvrzm@gmail.com>
What this PR does / why we need it:
Adds errorsource to the opensearch datasource.
Which issue(s) this PR fixes:
Fixes https://github.com/grafana/oss-plugin-partnerships/issues/1014
Special notes for your reviewer: