diff --git a/dax/controller/client/client.go b/dax/controller/client/client.go index 9f2adf3a6..38163f5d5 100644 --- a/dax/controller/client/client.go +++ b/dax/controller/client/client.go @@ -646,7 +646,7 @@ func (c *Client) RegisterNode(ctx context.Context, node *dax.Node) error { if resp.StatusCode != http.StatusOK { b, _ := io.ReadAll(resp.Body) - return errors.Errorf("registration request to %s status code: %d: %s", url, resp.StatusCode, b) + return errors.Wrapf(errors.UnmarshalJSON(b), "registration request to %s status code: %d: %s", url, resp.StatusCode, b) } return nil @@ -677,7 +677,7 @@ func (c *Client) CheckInNode(ctx context.Context, node *dax.Node) error { if resp.StatusCode != http.StatusOK { b, _ := io.ReadAll(resp.Body) - return errors.Errorf("status code: %d: %s", resp.StatusCode, b) + return errors.Wrapf(errors.UnmarshalJSON(b), "status code: %d: %s", resp.StatusCode, b) } return nil diff --git a/dax/controller/controller.go b/dax/controller/controller.go index c5244a8a2..7ce5376c0 100644 --- a/dax/controller/controller.go +++ b/dax/controller/controller.go @@ -267,6 +267,10 @@ func (c *Controller) RegisterNode(ctx context.Context, n *dax.Node) error { // from its list (perhaps due to a network fault) and therefore the node needs // to be re-registered. func (c *Controller) CheckInNode(ctx context.Context, n *dax.Node) error { + if n == nil || n.Address == "" { + return NewErrNodeKeyInvalid("") + } + tx, err := c.BoltDB.BeginTx(ctx, false) if err != nil { return errors.Wrap(err, "beginning tx") diff --git a/dax/controller/http/handler.go b/dax/controller/http/handler.go index ccda6e881..6071c89e5 100644 --- a/dax/controller/http/handler.go +++ b/dax/controller/http/handler.go @@ -615,7 +615,7 @@ func (s *server) postRegisterNode(w http.ResponseWriter, r *http.Request) { } if err := s.controller.RegisterNode(ctx, node); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + http.Error(w, errors.MarshalJSON(err), http.StatusBadRequest) return } @@ -700,7 +700,7 @@ func (s *server) postCheckInNode(w http.ResponseWriter, r *http.Request) { } if err := s.controller.CheckInNode(ctx, node); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + http.Error(w, errors.MarshalJSON(err), http.StatusBadRequest) return } diff --git a/dax/test/dax/dax_test.go b/dax/test/dax/dax_test.go index d3536183f..886c2247f 100644 --- a/dax/test/dax/dax_test.go +++ b/dax/test/dax/dax_test.go @@ -13,10 +13,12 @@ import ( featurebase "github.com/featurebasedb/featurebase/v3" "github.com/featurebasedb/featurebase/v3/dax" + "github.com/featurebasedb/featurebase/v3/dax/controller" controllerclient "github.com/featurebasedb/featurebase/v3/dax/controller/client" queryerclient "github.com/featurebasedb/featurebase/v3/dax/queryer/client" "github.com/featurebasedb/featurebase/v3/dax/server" "github.com/featurebasedb/featurebase/v3/dax/server/test" + "github.com/featurebasedb/featurebase/v3/errors" "github.com/featurebasedb/featurebase/v3/logger" "github.com/featurebasedb/featurebase/v3/sql3/test/defs" goerrors "github.com/pkg/errors" @@ -706,6 +708,46 @@ func TestDAXIntegration(t *testing.T) { assert.Equal(t, partitions2, nodes[2].Partitions) } }) + + t.Run("HTTPError", func(t *testing.T) { + mc := test.MustRunManagedCommand(t) + defer mc.Close() + + svcmgr := mc.Manage() + ctx := context.Background() + + t.Run("controller", func(t *testing.T) { + // Set up Controller client. + client := newWrappedControllerClient(controllerclient.New(svcmgr.Controller.Address(), svcmgr.Logger)) + + t.Run("Registrar", func(t *testing.T) { + t.Run("RegisterNode", func(t *testing.T) { + node := &dax.Node{} + err := client.RegisterNode(ctx, node) + if assert.Error(t, err) { + assert.True(t, errors.Is(err, controller.ErrCodeNodeKeyInvalid)) + } + }) + t.Run("CheckInNode", func(t *testing.T) { + node := &dax.Node{} + err := client.CheckInNode(ctx, node) + if assert.Error(t, err) { + assert.True(t, errors.Is(err, controller.ErrCodeNodeKeyInvalid)) + } + }) + }) + + t.Run("Schemar", func(t *testing.T) { + t.Run("CreateDatabase", func(t *testing.T) { + log.Printf("client: %+v", client) + err := client.CreateDatabase(ctx, qdb) + if assert.Error(t, err) { + assert.True(t, errors.Is(err, controller.ErrCodeNodeKeyInvalid)) + } + }) + }) + }) + }) } func dirIsEmpty(t *testing.T, name string) bool { diff --git a/dax/test/dax/wrappers_test.go b/dax/test/dax/wrappers_test.go new file mode 100644 index 000000000..a0b4913fa --- /dev/null +++ b/dax/test/dax/wrappers_test.go @@ -0,0 +1,65 @@ +package dax_test + +import ( + "context" + + "github.com/featurebasedb/featurebase/v3/dax" + "github.com/featurebasedb/featurebase/v3/dax/computer" + controllerclient "github.com/featurebasedb/featurebase/v3/dax/controller/client" +) + +var _ computer.Registrar = (*wrappedControllerClient)(nil) + +//var _ dax.Schemar = (*wrappedControllerClient)(nil) + +// wrappedControllerClient is a wrapper around the controller client which we +// use in tests to ensure that all of the methods for the computer.Registrar +// interface are covered by these tests. The idea being that if someone modifes +// the interface to include a new `Foo()` method, this test will no longer +// compile, and the developer will be directected here to add the appropriate +// tests. There's probably a more automated way to do this wil the `reflect` +// package, but that seems overly compilicated right now. +type wrappedControllerClient struct { + cli *controllerclient.Client + dax.Schemar +} + +func newWrappedControllerClient(cli *controllerclient.Client) *wrappedControllerClient { + return &wrappedControllerClient{ + cli: cli, + Schemar: dax.NewNopSchemar(), + } +} + +func (w *wrappedControllerClient) RegisterNode(ctx context.Context, node *dax.Node) error { + return w.cli.RegisterNode(ctx, node) +} + +func (w *wrappedControllerClient) CheckInNode(ctx context.Context, node *dax.Node) error { + return w.cli.CheckInNode(ctx, node) +} + +// Schemar + +/* +func (w *wrappedControllerClient) CreateDatabase(context.Context, *QualifiedDatabase) error +func (w *wrappedControllerClient) DropDatabase(context.Context, QualifiedDatabaseID) error + + DatabaseByName(ctx context.Context, orgID OrganizationID, dbname DatabaseName) (*QualifiedDatabase, error) + DatabaseByID(ctx context.Context, qdbid QualifiedDatabaseID) (*QualifiedDatabase, error) + + SetDatabaseOption(ctx context.Context, qdbid QualifiedDatabaseID, option string, value string) error + + Databases(context.Context, OrganizationID, ...DatabaseID) ([]*QualifiedDatabase, error) + + CreateTable(ctx context.Context, qtbl *QualifiedTable) error + DropTable(ctx context.Context, qtid QualifiedTableID) error + + TableByName(ctx context.Context, qdbid QualifiedDatabaseID, tname TableName) (*QualifiedTable, error) + TableByID(ctx context.Context, qtid QualifiedTableID) (*QualifiedTable, error) + + Tables(ctx context.Context, qdbid QualifiedDatabaseID, tids ...TableID) ([]*QualifiedTable, error) + + CreateField(ctx context.Context, qtid QualifiedTableID, fld *Field) error + DropField(ctx context.Context, qtid QualifiedTableID, fname FieldName) error +*/