Fix error source in errors with invalid JWT information #269
Conversation
| netErr, neErrOk := err.(net.Error) | ||
| if neErrOk && netErr.Timeout() || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { |
There was a problem hiding this comment.
We no longer have to take care of timeout errors as updated grafana-plugin-sdk-go handles that. You can also see that we are keeping the tests with checking for timeouts and it is correctly passing:
ivanahuckova
changed the title
| - name: unexported-return | ||
| - name: unexported-return | ||
| disabled: true | ||
| - name: redundant-import-alias |
There was a problem hiding this comment.
This was needed because of bumped grafana-plugin-sdk-go . It was complaining about Magefile.go and using named import in the code bellow, which is what we do in every data source and comes from create plugin.
//go:build mage
// +build mage
package main
import (
// mage:import
build "github.com/grafana/grafana-plugin-sdk-go/build"
)
// Default configures the default target.
var Default = build.BuildAll
| @@ -79,7 +81,7 @@ linters: | |||
| - dogsled | |||
| - dupl | |||
| - errcheck | |||
| - exportloopref | |||
| - copyloopvar | |||
There was a problem hiding this comment.
This change was recommended by ci as exportloopref is deprecated.
| if gridData, ok := item.(*sheets.GridData); ok { | ||
| return gridData, map[string]any{ | ||
| "hit": true, | ||
| "expires": expires.Unix(), | ||
| }, nil | ||
| } | ||
| return nil, nil, errors.New("invalid cache item not type of *sheets.GridData") |
There was a problem hiding this comment.
There is no change in this besides checking if item is actually GridData. But linter was complaining that we weren't checking if item is GridData, so I added it.
| if spreadsheet, ok := args.Get(0).(*sheets.Spreadsheet); ok { | ||
| return spreadsheet, args.Error(1) | ||
| } | ||
| return nil, args.Error(1) |
There was a problem hiding this comment.
Another fix for linter complaining that we aren't checking if spreadsheet is actually *sheets.Spreadsheet.
| @@ -241,7 +284,8 @@ func TestGooglesheets(t *testing.T) { | |||
| }) | |||
|
|
|||
| t.Run("meta warnings field is populated correctly", func(t *testing.T) { | |||
| warnings := meta["warnings"].([]string) | |||
| warnings, ok := meta["warnings"].([]string) | |||
There was a problem hiding this comment.
Last linter warning fixed.
This PR takes care of false-positive plugin errors identified in https://ops.grafana-ops.net/goto/BHCltU6SR?orgId=1. The main issue is that if users input invalid Google JWT information - for example incorrect email:
The returned error was plugin error as we weren't handling that case. This PR fixes it by checking for
*oauth2.RetrieveErrorand based on status code from that error sets the error source. It also removes a logic to check for timeouts, as bumpedgrafana-plugin-sdk-gotakes care of it. We have tests in place for this and they are passing even with removed logic, assdkcorrectly handles it.Now this is fixed to downstream:
Also I had to fix some lint issues (see https://drone.grafana.net/grafana/google-sheets-datasource/447/1/8) - I have added a comments to them bellow.