Errors: Make errors the same in dev as prod (#77366)

When running in dev mode, error messages would contain an additional "error" property alongside "message". Since this causes confusion, that has been removed and now error messages are the same both modes (using "message").
This commit is contained in:
Kyle Brandt 2023-10-30 14:06:26 -04:00 committed by GitHub
parent 40df27a4da
commit e4d1fdc3d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 16 additions and 49 deletions

View File

@ -230,7 +230,7 @@ func TestAdminAPIEndpoint(t *testing.T) {
respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
require.NoError(t, err)
assert.Equal(t, "user already exists", respJSON.Get("error").MustString())
assert.Equal(t, "User with email '' or username 'existing@example.com' already exists", respJSON.Get("message").MustString())
})
})
}

View File

@ -3,7 +3,6 @@ package api
import (
"bytes"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
@ -261,7 +260,7 @@ func TestSetFeatureToggles(t *testing.T) {
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, fmt.Sprintf("invalid toggle passed in: %s", featuremgmt.FlagFeatureToggleAdminPage), p["error"])
assert.Equal(t, "invalid toggle passed in", p["message"])
})
t.Run("because it is not GA or Deprecated", func(t *testing.T) {
@ -274,7 +273,7 @@ func TestSetFeatureToggles(t *testing.T) {
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "invalid toggle passed in: toggle2", p["error"])
assert.Equal(t, "invalid toggle passed in", p["message"])
})
t.Run("because it is configured to be read-only", func(t *testing.T) {
@ -287,7 +286,7 @@ func TestSetFeatureToggles(t *testing.T) {
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "invalid toggle passed in: toggle3", p["error"])
assert.Equal(t, "invalid toggle passed in", p["message"])
})
})

View File

@ -162,7 +162,7 @@ func TestAPIEndpoint_Metrics_PluginDecryptionFailure(t *testing.T) {
var resObj secretsErrorResponseBody
err = json.Unmarshal(buf.Bytes(), &resObj)
require.NoError(t, err)
require.Equal(t, "unknown error", resObj.Error)
require.Equal(t, "", resObj.Error)
require.Contains(t, resObj.Message, "Secrets Plugin error:")
})
}
@ -264,12 +264,12 @@ func TestDataSourceQueryError(t *testing.T) {
{
request: reqDatasourceByUidNotFound,
expectedStatus: http.StatusNotFound,
expectedBody: `{"error":"data source not found","message":"Data source not found","traceID":""}`,
expectedBody: `{"message":"Data source not found","traceID":""}`,
},
{
request: reqDatasourceByIdNotFound,
expectedStatus: http.StatusNotFound,
expectedBody: `{"error":"data source not found","message":"Data source not found","traceID":""}`,
expectedBody: `{"message":"Data source not found","traceID":""}`,
},
}

View File

@ -128,7 +128,7 @@ func TestCallResource(t *testing.T) {
_, err = io.Copy(body, resp.Body)
require.NoError(t, err)
expectedBody := `{ "error": "something went wrong", "message": "Failed to call resource", "traceID": "" }`
expectedBody := `{ "message": "Failed to call resource", "traceID": "" }`
require.JSONEq(t, expectedBody, body.String())
require.NoError(t, resp.Body.Close())
require.Equal(t, 500, resp.StatusCode)

View File

@ -15,7 +15,6 @@ import (
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/middleware/requestmeta"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util/errutil"
)
@ -253,12 +252,6 @@ func Error(status int, message string, err error) *NormalResponse {
data["message"] = message
}
if err != nil {
if setting.Env != setting.Prod {
data["error"] = err.Error()
}
}
resp := JSON(status, data)
if err != nil {

View File

@ -46,11 +46,9 @@ func (ctx *ReqContext) Handle(cfg *setting.Cfg, status int, title string, err er
Theme string
ErrorMsg error
}{title, "Grafana", cfg.AppSubURL, "dark", nil}
if err != nil {
ctx.Logger.Error(title, "error", err)
if setting.Env != setting.Prod {
data.ErrorMsg = err
}
}
ctx.HTML(status, cfg.ErrTemplateName, data)
@ -75,10 +73,6 @@ func (ctx *ReqContext) JsonApiErr(status int, message string, err error) {
} else {
ctx.Logger.Warn(message, "error", err, "traceID", traceID)
}
if setting.Env != setting.Prod {
resp["error"] = err.Error()
}
}
switch status {

View File

@ -69,19 +69,16 @@ func TestDataProxy(t *testing.T) {
// Tests request to datasource proxy service
func TestDatasourceProxy_proxyDatasourceRequest(t *testing.T) {
tcs := []struct {
name string
dsURL string
expectedErrorMsg string
name string
dsURL string
}{
{
name: "Empty datasource URL will return a 400 HTTP status code",
dsURL: "",
expectedErrorMsg: "validation of data source URL \"\" failed: empty URL string",
name: "Empty datasource URL will return a 400 HTTP status code",
dsURL: "",
},
{
name: "Invalid datasource URL will return a 400 HTTP status code",
dsURL: "://host/path",
expectedErrorMsg: "validation of data source URL \"://host/path\" failed: parse \"://host/path\": missing protocol scheme",
name: "Invalid datasource URL will return a 400 HTTP status code",
dsURL: "://host/path",
},
}
for _, tc := range tcs {
@ -124,7 +121,6 @@ func TestDatasourceProxy_proxyDatasourceRequest(t *testing.T) {
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
require.Equal(t, fmt.Sprintf("Invalid data source URL: %q", tc.dsURL), jsonBody["message"])
require.Equal(t, tc.expectedErrorMsg, jsonBody["error"])
})
}
}

View File

@ -180,7 +180,6 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) {
var resMap map[string]interface{}
err = json.Unmarshal(bodyBytes, &resMap)
assert.NoError(t, err)
assert.Equal(t, "unable to find organization with ID '2'", resMap["error"])
assert.Equal(t, "An organization was not found - Please verify your LDAP configuration", resMap["message"])
}
@ -499,7 +498,6 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) {
var resMap map[string]interface{}
err = json.Unmarshal(bodyBytes, &resMap)
assert.NoError(t, err)
assert.Equal(t, "did not find a user", resMap["error"])
assert.Equal(t, "Refusing to sync grafana super admin \"ldap-daniel\" - it would be disabled", resMap["message"])
}

View File

@ -77,7 +77,7 @@ func TestProvisioningApi(t *testing.T) {
response := sut.RoutePutPolicyTree(&rc, tree)
require.Equal(t, 400, response.Status())
expBody := `{"error":"invalid object specification: invalid policy tree","message":"invalid object specification: invalid policy tree"}`
expBody := `{"message":"invalid object specification: invalid policy tree"}`
require.Equal(t, expBody, string(response.Body()))
})
})

View File

@ -133,7 +133,6 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Data source not found", response.Message)
require.Equal(t, correlations.ErrSourceDataSourceDoesNotExists.Error(), response.Error)
require.NoError(t, res.Body.Close())
})
@ -161,7 +160,6 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Data source not found", response.Message)
require.Equal(t, correlations.ErrTargetDataSourceDoesNotExists.Error(), response.Error)
require.NoError(t, res.Body.Close())
})
@ -357,8 +355,6 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Contains(t, response.Message, "bad request data")
require.Contains(t, response.Error, correlations.ErrInvalidConfigType.Error())
require.Contains(t, response.Error, configType)
require.NoError(t, res.Body.Close())
})

View File

@ -106,7 +106,6 @@ func TestIntegrationDeleteCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Data source not found", response.Message)
require.Equal(t, correlations.ErrSourceDataSourceDoesNotExists.Error(), response.Error)
require.NoError(t, res.Body.Close())
})
@ -126,7 +125,6 @@ func TestIntegrationDeleteCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Correlation not found", response.Message)
require.Equal(t, correlations.ErrCorrelationNotFound.Error(), response.Error)
require.NoError(t, res.Body.Close())
})
@ -153,7 +151,6 @@ func TestIntegrationDeleteCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Correlation can only be edited via provisioning", response.Message)
require.Equal(t, correlations.ErrCorrelationReadOnly.Error(), response.Error)
require.NoError(t, res.Body.Close())
})

View File

@ -100,7 +100,6 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Data source not found", response.Message)
require.Equal(t, correlations.ErrSourceDataSourceDoesNotExists.Error(), response.Error)
require.NoError(t, res.Body.Close())
})
@ -123,7 +122,6 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Correlation not found", response.Message)
require.Equal(t, correlations.ErrCorrelationNotFound.Error(), response.Error)
require.NoError(t, res.Body.Close())
})
@ -153,7 +151,6 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Correlation can only be edited via provisioning", response.Message)
require.Equal(t, correlations.ErrCorrelationReadOnly.Error(), response.Error)
require.NoError(t, res.Body.Close())
})
@ -181,7 +178,6 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "At least one of label, description or config is required", response.Message)
require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
require.NoError(t, res.Body.Close())
// empty body
@ -199,7 +195,6 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "At least one of label, description or config is required", response.Message)
require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
require.NoError(t, res.Body.Close())
// all set to null
@ -221,7 +216,6 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "At least one of label, description or config is required", response.Message)
require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
require.NoError(t, res.Body.Close())
})