SDK error revamp (#432)

SDK methods now return 1 error.  To differentiate between Client errors and Server errors I've also implemented the AsServerError function which returns an *api.Error if it is an error originating from the remote server or nil otherwise.

api/scopes/scope.gen.go

@@ -9,8 +9,6 @@ import (
 	"net/url"
 	"time"
 
-	"github.com/kr/pretty"
-
 	"github.com/hashicorp/boundary/api"
 )
 
@@ -107,22 +105,22 @@ func (c *Client) ApiClient() *api.Client {
 	return c.client
 }
 
-func (c *Client) Create(ctx context.Context, scopeId string, opt ...Option) (*ScopeCreateResult, *api.Error, error) {
+func (c *Client) Create(ctx context.Context, scopeId string, opt ...Option) (*ScopeCreateResult, error) {
 	if scopeId == "" {
-		return nil, nil, fmt.Errorf("empty scopeId value passed into Create request")
+		return nil, fmt.Errorf("empty scopeId value passed into Create request")
 	}
 
 	opts, apiOpts := getOpts(opt...)
 
 	if c.client == nil {
-		return nil, nil, fmt.Errorf("nil client")
+		return nil, fmt.Errorf("nil client")
 	}
 
 	opts.postMap["scope_id"] = scopeId
 
 	req, err := c.client.NewRequest(ctx, "POST", "scopes", opts.postMap, apiOpts...)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error creating Create request: %w", err)
+		return nil, fmt.Errorf("error creating Create request: %w", err)
 	}
 
 	if len(opts.queryMap) > 0 {
@@ -135,36 +133,36 @@ func (c *Client) Create(ctx context.Context, scopeId string, opt ...Option) (*Sc
 
 	resp, err := c.client.Do(req)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error performing client request during Create call: %w", err)
+		return nil, fmt.Errorf("error performing client request during Create call: %w", err)
 	}
 
 	target := new(ScopeCreateResult)
 	target.Item = new(Scope)
 	apiErr, err := resp.Decode(target.Item)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error decoding Create response: %w", err)
+		return nil, fmt.Errorf("error decoding Create response: %w", err)
 	}
 	if apiErr != nil {
-		return nil, apiErr, nil
+		return nil, apiErr
 	}
 	target.responseBody = resp.Body
 	target.responseMap = resp.Map
-	return target, apiErr, nil
+	return target, nil
 }
 
-func (c *Client) Read(ctx context.Context, scopeId string, opt ...Option) (*ScopeReadResult, *api.Error, error) {
+func (c *Client) Read(ctx context.Context, scopeId string, opt ...Option) (*ScopeReadResult, error) {
 	if scopeId == "" {
-		return nil, nil, fmt.Errorf("empty scopeId value passed into Read request")
+		return nil, fmt.Errorf("empty scopeId value passed into Read request")
 	}
 	if c.client == nil {
-		return nil, nil, fmt.Errorf("nil client")
+		return nil, fmt.Errorf("nil client")
 	}
 
 	opts, apiOpts := getOpts(opt...)
 
 	req, err := c.client.NewRequest(ctx, "GET", fmt.Sprintf("scopes/%s", scopeId), nil, apiOpts...)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error creating Read request: %w", err)
+		return nil, fmt.Errorf("error creating Read request: %w", err)
 	}
 
 	if len(opts.queryMap) > 0 {
@@ -177,49 +175,49 @@ func (c *Client) Read(ctx context.Context, scopeId string, opt ...Option) (*Scop
 
 	resp, err := c.client.Do(req)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error performing client request during Read call: %w", err)
+		return nil, fmt.Errorf("error performing client request during Read call: %w", err)
 	}
 
 	target := new(ScopeReadResult)
 	target.Item = new(Scope)
 	apiErr, err := resp.Decode(target.Item)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error decoding Read response: %w", err)
+		return nil, fmt.Errorf("error decoding Read response: %w", err)
 	}
 	if apiErr != nil {
-		return nil, apiErr, nil
+		return nil, apiErr
 	}
 	target.responseBody = resp.Body
 	target.responseMap = resp.Map
-	return target, apiErr, nil
+	return target, nil
 }
 
-func (c *Client) Update(ctx context.Context, scopeId string, version uint32, opt ...Option) (*ScopeUpdateResult, *api.Error, error) {
+func (c *Client) Update(ctx context.Context, scopeId string, version uint32, opt ...Option) (*ScopeUpdateResult, error) {
 	if scopeId == "" {
-		return nil, nil, fmt.Errorf("empty scopeId value passed into Update request")
+		return nil, fmt.Errorf("empty scopeId value passed into Update request")
 	}
 	if c.client == nil {
-		return nil, nil, fmt.Errorf("nil client")
+		return nil, fmt.Errorf("nil client")
 	}
 
 	opts, apiOpts := getOpts(opt...)
 
 	if version == 0 {
 		if !opts.withAutomaticVersioning {
-			return nil, nil, errors.New("zero version number passed into Update request and automatic versioning not specified")
+			return nil, errors.New("zero version number passed into Update request and automatic versioning not specified")
 		}
-		existingTarget, existingApiErr, existingErr := c.Read(ctx, scopeId, opt...)
+		existingTarget, existingErr := c.Read(ctx, scopeId, opt...)
 		if existingErr != nil {
-			return nil, nil, fmt.Errorf("error performing initial check-and-set read: %w", existingErr)
-		}
-		if existingApiErr != nil {
-			return nil, nil, fmt.Errorf("error from controller when performing initial check-and-set read: %s", pretty.Sprint(existingApiErr))
+			if api.AsServerError(existingErr) != nil {
+				return nil, fmt.Errorf("error from controller when performing initial check-and-set read: %w", existingErr)
+			}
+			return nil, fmt.Errorf("error performing initial check-and-set read: %w", existingErr)
 		}
 		if existingTarget == nil {
-			return nil, nil, errors.New("nil resource response found when performing initial check-and-set read")
+			return nil, errors.New("nil resource response found when performing initial check-and-set read")
 		}
 		if existingTarget.Item == nil {
-			return nil, nil, errors.New("nil resource found when performing initial check-and-set read")
+			return nil, errors.New("nil resource found when performing initial check-and-set read")
 		}
 		version = existingTarget.Item.Version
 	}
@@ -228,7 +226,7 @@ func (c *Client) Update(ctx context.Context, scopeId string, version uint32, opt
 
 	req, err := c.client.NewRequest(ctx, "PATCH", fmt.Sprintf("scopes/%s", scopeId), opts.postMap, apiOpts...)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error creating Update request: %w", err)
+		return nil, fmt.Errorf("error creating Update request: %w", err)
 	}
 
 	if len(opts.queryMap) > 0 {
@@ -241,36 +239,36 @@ func (c *Client) Update(ctx context.Context, scopeId string, version uint32, opt
 
 	resp, err := c.client.Do(req)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error performing client request during Update call: %w", err)
+		return nil, fmt.Errorf("error performing client request during Update call: %w", err)
 	}
 
 	target := new(ScopeUpdateResult)
 	target.Item = new(Scope)
 	apiErr, err := resp.Decode(target.Item)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error decoding Update response: %w", err)
+		return nil, fmt.Errorf("error decoding Update response: %w", err)
 	}
 	if apiErr != nil {
-		return nil, apiErr, nil
+		return nil, apiErr
 	}
 	target.responseBody = resp.Body
 	target.responseMap = resp.Map
-	return target, apiErr, nil
+	return target, nil
 }
 
-func (c *Client) Delete(ctx context.Context, scopeId string, opt ...Option) (*ScopeDeleteResult, *api.Error, error) {
+func (c *Client) Delete(ctx context.Context, scopeId string, opt ...Option) (*ScopeDeleteResult, error) {
 	if scopeId == "" {
-		return nil, nil, fmt.Errorf("empty scopeId value passed into Delete request")
+		return nil, fmt.Errorf("empty scopeId value passed into Delete request")
 	}
 	if c.client == nil {
-		return nil, nil, fmt.Errorf("nil client")
+		return nil, fmt.Errorf("nil client")
 	}
 
 	opts, apiOpts := getOpts(opt...)
 
 	req, err := c.client.NewRequest(ctx, "DELETE", fmt.Sprintf("scopes/%s", scopeId), nil, apiOpts...)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error creating Delete request: %w", err)
+		return nil, fmt.Errorf("error creating Delete request: %w", err)
 	}
 
 	if len(opts.queryMap) > 0 {
@@ -283,30 +281,30 @@ func (c *Client) Delete(ctx context.Context, scopeId string, opt ...Option) (*Sc
 
 	resp, err := c.client.Do(req)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error performing client request during Delete call: %w", err)
+		return nil, fmt.Errorf("error performing client request during Delete call: %w", err)
 	}
 
 	apiErr, err := resp.Decode(nil)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error decoding Delete response: %w", err)
+		return nil, fmt.Errorf("error decoding Delete response: %w", err)
 	}
 	if apiErr != nil {
-		return nil, apiErr, nil
+		return nil, apiErr
 	}
 
 	target := &ScopeDeleteResult{
 		responseBody: resp.Body,
 		responseMap:  resp.Map,
 	}
-	return target, nil, nil
+	return target, nil
 }
 
-func (c *Client) List(ctx context.Context, scopeId string, opt ...Option) (*ScopeListResult, *api.Error, error) {
+func (c *Client) List(ctx context.Context, scopeId string, opt ...Option) (*ScopeListResult, error) {
 	if scopeId == "" {
-		return nil, nil, fmt.Errorf("empty scopeId value passed into List request")
+		return nil, fmt.Errorf("empty scopeId value passed into List request")
 	}
 	if c.client == nil {
-		return nil, nil, fmt.Errorf("nil client")
+		return nil, fmt.Errorf("nil client")
 	}
 
 	opts, apiOpts := getOpts(opt...)
@@ -314,7 +312,7 @@ func (c *Client) List(ctx context.Context, scopeId string, opt ...Option) (*Scop
 
 	req, err := c.client.NewRequest(ctx, "GET", "scopes", nil, apiOpts...)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error creating List request: %w", err)
+		return nil, fmt.Errorf("error creating List request: %w", err)
 	}
 
 	if len(opts.queryMap) > 0 {
@@ -327,18 +325,18 @@ func (c *Client) List(ctx context.Context, scopeId string, opt ...Option) (*Scop
 
 	resp, err := c.client.Do(req)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error performing client request during List call: %w", err)
+		return nil, fmt.Errorf("error performing client request during List call: %w", err)
 	}
 
 	target := new(ScopeListResult)
 	apiErr, err := resp.Decode(target)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error decoding List response: %w", err)
+		return nil, fmt.Errorf("error decoding List response: %w", err)
 	}
 	if apiErr != nil {
-		return nil, apiErr, nil
+		return nil, apiErr
 	}
 	target.responseBody = resp.Body
 	target.responseMap = resp.Map
-	return target, apiErr, nil
+	return target, nil
 }

api/scopes/scope_test.go

@@ -25,21 +25,18 @@ func TestList(t *testing.T) {
 
 	scps := scopes.NewClient(client)
 
-	pl, apiErr, err := scps.List(tc.Context(), org.GetPublicId())
+	pl, err := scps.List(tc.Context(), org.GetPublicId())
 	require.NoError(err)
-	assert.Nil(apiErr)
 	assert.Empty(pl.Items)
 
 	expected := make([]*scopes.Scope, 10)
 	for i := 0; i < 10; i++ {
-		scr, apiErr, err := scps.Create(tc.Context(), org.GetPublicId(), scopes.WithName(fmt.Sprintf("%d", i)))
+		scr, err := scps.Create(tc.Context(), org.GetPublicId(), scopes.WithName(fmt.Sprintf("%d", i)))
 		require.NoError(err)
-		assert.Nil(apiErr)
 		expected[i] = scr.Item
 	}
-	pl, apiErr, err = scps.List(tc.Context(), org.GetPublicId())
+	pl, err = scps.List(tc.Context(), org.GetPublicId())
 	require.NoError(err)
-	assert.Nil(apiErr)
 	assert.ElementsMatch(comparableSlice(expected), comparableSlice(pl.Items))
 }
 
@@ -71,9 +68,8 @@ func TestCrud(t *testing.T) {
 
 	scps := scopes.NewClient(client)
 
-	checkProject := func(step string, s *scopes.Scope, apiErr *api.Error, err error, wantedName string, wantedVersion uint32) {
+	checkProject := func(step string, s *scopes.Scope, err error, wantedName string, wantedVersion uint32) {
 		require.NoError(err, step)
-		assert.Nil(apiErr, step)
 		assert.NotNil(s, "returned project", step)
 		gotName := ""
 		if s.Name != "" {
@@ -83,29 +79,28 @@ func TestCrud(t *testing.T) {
 		assert.Equal(wantedVersion, s.Version)
 	}
 
-	s, apiErr, err := scps.Create(tc.Context(), org.GetPublicId(), scopes.WithName("foo"))
-	checkProject("create", s.Item, apiErr, err, "foo", 1)
+	s, err := scps.Create(tc.Context(), org.GetPublicId(), scopes.WithName("foo"))
+	checkProject("create", s.Item, err, "foo", 1)
 
-	s, apiErr, err = scps.Read(tc.Context(), s.Item.Id)
-	checkProject("read", s.Item, apiErr, err, "foo", 1)
+	s, err = scps.Read(tc.Context(), s.Item.Id)
+	checkProject("read", s.Item, err, "foo", 1)
 
-	s, apiErr, err = scps.Update(tc.Context(), s.Item.Id, s.Item.Version, scopes.WithName("bar"))
-	checkProject("update", s.Item, apiErr, err, "bar", 2)
+	s, err = scps.Update(tc.Context(), s.Item.Id, s.Item.Version, scopes.WithName("bar"))
+	checkProject("update", s.Item, err, "bar", 2)
 
-	s, apiErr, err = scps.Update(tc.Context(), s.Item.Id, s.Item.Version, scopes.DefaultName())
-	checkProject("update, unset name", s.Item, apiErr, err, "", 3)
+	s, err = scps.Update(tc.Context(), s.Item.Id, s.Item.Version, scopes.DefaultName())
+	checkProject("update, unset name", s.Item, err, "", 3)
 
-	_, apiErr, err = scps.Delete(tc.Context(), s.Item.Id)
+	_, err = scps.Delete(tc.Context(), s.Item.Id)
 	require.NoError(err)
-	assert.Nil(apiErr)
 
-	_, apiErr, err = scps.Delete(tc.Context(), s.Item.Id)
-	require.NoError(err)
+	_, err = scps.Delete(tc.Context(), s.Item.Id)
+	require.Error(err)
+	apiErr := api.AsServerError(err)
 	assert.NotNil(apiErr)
 	assert.EqualValues(http.StatusNotFound, apiErr.Status)
 }
 
-// TODO: Get better coverage for expected errors and error formats.
 func TestErrors(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 	tc := controller.NewTestController(t, nil)
@@ -118,25 +113,26 @@ func TestErrors(t *testing.T) {
 
 	scps := scopes.NewClient(client)
 
-	createdProj, apiErr, err := scps.Create(tc.Context(), org.GetPublicId())
+	createdProj, err := scps.Create(tc.Context(), org.GetPublicId())
 	require.NoError(err)
 	assert.NotNil(createdProj)
-	assert.Nil(apiErr)
 
 	// Updating the wrong version should fail.
-	_, apiErr, err = scps.Update(tc.Context(), createdProj.Item.Id, 73, scopes.WithName("anything"))
-	require.NoError(err)
+	_, err = scps.Update(tc.Context(), createdProj.Item.Id, 73, scopes.WithName("anything"))
+	require.Error(err)
+	apiErr := api.AsServerError(err)
 	assert.NotNil(apiErr)
 	assert.EqualValues(http.StatusNotFound, apiErr.Status)
 
-	_, apiErr, err = scps.Read(tc.Context(), "p_doesntexis")
-	require.NoError(err)
-	// TODO: Should this be nil instead of just a Project that has no values set
+	_, err = scps.Read(tc.Context(), "p_doesntexis")
+	require.Error(err)
+	apiErr = api.AsServerError(err)
 	assert.NotNil(apiErr)
 	assert.EqualValues(http.StatusNotFound, apiErr.Status)
 
-	_, apiErr, err = scps.Read(tc.Context(), "invalid id")
-	assert.NoError(err)
+	_, err = scps.Read(tc.Context(), "invalid id")
+	assert.Error(err)
+	apiErr = api.AsServerError(err)
 	assert.NotNil(apiErr)
 	assert.EqualValues(http.StatusBadRequest, apiErr.Status)
 }