Skip to content

Commit

Permalink
[query] Fix aggregated namespace header to be bad request instead of …
Browse files Browse the repository at this point in the history
…internal server error
  • Loading branch information
robskillington committed Jan 8, 2021
1 parent 7917646 commit da5e843
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
7 changes: 5 additions & 2 deletions src/query/storage/m3/cluster_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/m3db/m3/src/query/storage"
"github.com/m3db/m3/src/query/storage/m3/consolidators"
"github.com/m3db/m3/src/query/storage/m3/storagemetadata"
xerrors "github.com/m3db/m3/src/x/errors"
)

type unaggregatedNamespaceType uint8
Expand Down Expand Up @@ -346,15 +347,17 @@ func resolveClusterNamespacesForQueryWithRestrictQueryOptions(
Resolution: restrict.StoragePolicy.Resolution().Window,
})
if !ok {
return result(nil,
err := xerrors.NewInvalidParamsError(
fmt.Errorf("could not find namespace for storage policy: %v",
restrict.StoragePolicy.String()))
return result(nil, err)
}

return result(ns, nil)
default:
return result(nil,
err := xerrors.NewInvalidParamsError(
fmt.Errorf("unrecognized metrics type: %v", restrict.MetricsType))
return result(nil, err)
}
}

Expand Down
26 changes: 16 additions & 10 deletions src/query/storage/m3/cluster_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/m3db/m3/src/query/storage"
"github.com/m3db/m3/src/query/storage/m3/consolidators"
"github.com/m3db/m3/src/query/storage/m3/storagemetadata"
xerrors "github.com/m3db/m3/src/x/errors"
"github.com/m3db/m3/src/x/ident"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -162,14 +163,15 @@ func generateClusters(t *testing.T, ctrl *gomock.Controller) Clusters {
}

var testCases = []struct {
name string
queryLength time.Duration
opts *storage.FanoutOptions
restrict *storage.RestrictQueryOptions
expectedType consolidators.QueryFanoutType
expectedClusterNames []string
expectedErr error
expectedErrContains string
name string
queryLength time.Duration
opts *storage.FanoutOptions
restrict *storage.RestrictQueryOptions
expectedType consolidators.QueryFanoutType
expectedClusterNames []string
expectedErr error
expectedErrContains string
expectedErrInvalidParams bool
}{
{
name: "all disabled",
Expand Down Expand Up @@ -355,7 +357,8 @@ var testCases = []struct {
MetricsType: storagemetadata.UnknownMetricsType,
},
},
expectedErrContains: "unrecognized metrics type:",
expectedErrContains: "unrecognized metrics type:",
expectedErrInvalidParams: true,
},
{
name: "restrict with unknown storage policy",
Expand All @@ -366,7 +369,8 @@ var testCases = []struct {
StoragePolicy: policy.MustParseStoragePolicy("1s:100d"),
},
},
expectedErrContains: "could not find namespace for storage policy:",
expectedErrContains: "could not find namespace for storage policy:",
expectedErrInvalidParams: true,
},
}

Expand Down Expand Up @@ -398,6 +402,8 @@ func TestResolveClusterNamespacesForQueryWithOptions(t *testing.T) {
assert.Error(t, err)
assert.True(t, strings.Contains(err.Error(), substr))
assert.Nil(t, clusters)
invalidParams := xerrors.IsInvalidParams(err)
assert.Equal(t, tt.expectedErrInvalidParams, invalidParams)
return
}

Expand Down

0 comments on commit da5e843

Please sign in to comment.