fix!: Diagnostic cleanup

cqproto/grpc.go

@@ -172,7 +172,7 @@ func (g *GRPCServer) ConfigureProvider(ctx context.Context, request *internal.Co
 		return nil, err
 	}
 	return &internal.ConfigureProvider_Response{
-		Error:       resp.Diagnostics.Error(), // For backwards compatibility
+		Error:       resp.Diagnostics.DeprecatedToError(), // For backwards compatibility
 		Diagnostics: diagnosticsToProto(resp.Diagnostics),
 	}, nil
 }

database/postgres/pgdatabase.go

@@ -46,7 +46,7 @@ func NewPgDatabase(ctx context.Context, logger hclog.Logger, dsn string, sd sche
 }
 
 // Insert inserts all resources to given table, table and resources are assumed from same table.
-func (p PgDatabase) Insert(ctx context.Context, t *schema.Table, resources schema.Resources, shouldCascade bool) error {
+func (p PgDatabase) Insert(ctx context.Context, t *schema.Table, resources schema.Resources, shouldCascade bool) diag.Diagnostics {
 	if len(resources) == 0 {
 		return nil
 	}
@@ -57,18 +57,18 @@ func (p PgDatabase) Insert(ctx context.Context, t *schema.Table, resources schem
 	sqlStmt := psql.Insert(t.Name).Columns(cols...)
 	for _, res := range resources {
 		if res.TableName() != t.Name {
-			return fmt.Errorf("resource table expected %s got %s", t.Name, res.TableName())
+			return diag.FromError(fmt.Errorf("resource table expected %s got %s", t.Name, res.TableName()), diag.INTERNAL)
 		}
 		values, err := res.Values()
 		if err != nil {
-			return fmt.Errorf("table %s insert failed %w", t.Name, err)
+			return diag.FromError(fmt.Errorf("table %s insert failed %w", t.Name, err), diag.INTERNAL)
 		}
 		sqlStmt = sqlStmt.Values(values...)
 	}
 
 	s, args, err := sqlStmt.ToSql()
 	if err != nil {
-		return diag.NewBaseError(err, diag.DATABASE, diag.WithResourceName(t.Name), diag.WithSummary("bad insert SQL statement created"), diag.WithDetails("SQL statement %q is invalid", s))
+		return diag.Diagnostics{diag.NewBaseError(err, diag.DATABASE, diag.WithResourceName(t.Name), diag.WithSummary("bad insert SQL statement created"), diag.WithDetails("SQL statement %q is invalid", s))}
 	}
 
 	err = p.pool.BeginTxFunc(ctx, pgx.TxOptions{
@@ -97,9 +97,9 @@ func (p PgDatabase) Insert(ctx context.Context, t *schema.Table, resources schem
 		if pgerrcode.IsIntegrityConstraintViolation(pgErr.Code) {
 			p.log.Debug("insert integrity violation error", "constraint", pgErr.ConstraintName, "errMsg", pgErr.Message)
 		}
-		return diag.NewBaseError(err, diag.DATABASE, diag.WithResourceName(t.Name), diag.WithSummary("failed to insert to table %q", t.Name), diag.WithDetails("%s", pgErr.Message))
+		return diag.Diagnostics{diag.NewBaseError(err, diag.DATABASE, diag.WithResourceName(t.Name), diag.WithSummary("failed to insert to table %q", t.Name), diag.WithDetails("%s", pgErr.Message))}
 	}
-	return diag.NewBaseError(err, diag.DATABASE, diag.WithResourceName(t.Name))
+	return diag.Diagnostics{diag.NewBaseError(err, diag.DATABASE, diag.WithResourceName(t.Name))}
 }
 
 // CopyFrom copies all resources from []*Resource

provider/diag/diagnostic.go

@@ -6,7 +6,6 @@ type Severity int
 // nolint:revive
 type Type int
 type Diagnostic interface {
-	error
 	Severity() Severity
 	Type() Type
 	Description() Description

provider/diag/diagnostics.go

@@ -15,7 +15,10 @@ type Diagnostics []Diagnostic
 
 var _ sort.Interface = (*Diagnostics)(nil)
 
-func (diags Diagnostics) Error() string {
+// Deprecated! Only here for backward compatibility (specifically in the gRPC protocol). Do not use unless
+//  you are 100% sure you need this.
+// Converts a list of diagnostics into an error.
+func (diags Diagnostics) DeprecatedToError() string {
 	switch {
 	case len(diags) == 0:
 		// should never happen, since we don't create this wrapper if
@@ -114,7 +117,7 @@ func (diags Diagnostics) Squash() Diagnostics {
 			}
 		}
 
-		key := fmt.Sprintf("%s_%s_%s_%d_%d", reflect.ValueOf(keygen).Type().String(), keygen.Error(), keygen.Description().Resource, keygen.Severity(), keygen.Type())
+		key := fmt.Sprintf("%s_%s_%s_%d_%d", reflect.ValueOf(keygen).Type().String(), keygen.Description().Summary, keygen.Description().Resource, keygen.Severity(), keygen.Type())
 		if sd, ok := dd[key]; ok {
 			sd.count += CountDiag(d)
 			continue

provider/diag/diagnostics_test.go

@@ -23,7 +23,6 @@ func TestDiagnostics_Squash(t *testing.T) {
 			},
 			Want: FlatDiags{
 				{
-					Err:      "error test",
 					Resource: "a",
 					Type:     RESOLVING,
 					Severity: ERROR,
@@ -46,7 +45,6 @@ func TestDiagnostics_Squash(t *testing.T) {
 			},
 			Want: FlatDiags{
 				{
-					Err:      "error test",
 					Resource: "a",
 					Type:     RESOLVING,
 					Severity: ERROR,
@@ -58,7 +56,6 @@ func TestDiagnostics_Squash(t *testing.T) {
 					},
 				},
 				{
-					Err:      "error test2",
 					Resource: "a",
 					Type:     RESOLVING,
 					Severity: ERROR,
@@ -79,7 +76,6 @@ func TestDiagnostics_Squash(t *testing.T) {
 			},
 			Want: FlatDiags{
 				{
-					Err:      "error test",
 					Resource: "a",
 					Type:     RESOLVING,
 					Severity: ERROR,
@@ -91,7 +87,6 @@ func TestDiagnostics_Squash(t *testing.T) {
 					},
 				},
 				{
-					Err:      "error test",
 					Resource: "b",
 					Type:     RESOLVING,
 					Severity: ERROR,
@@ -112,7 +107,6 @@ func TestDiagnostics_Squash(t *testing.T) {
 			},
 			Want: FlatDiags{
 				{
-					Err:      "error test",
 					Resource: "a",
 					Type:     RESOLVING,
 					Severity: WARNING,
@@ -124,7 +118,6 @@ func TestDiagnostics_Squash(t *testing.T) {
 					},
 				},
 				{
-					Err:      "error test",
 					Resource: "a",
 					Type:     RESOLVING,
 					Severity: ERROR,
@@ -162,7 +155,6 @@ func TestDiagnostics_SquashRedactable(t *testing.T) {
 
 	assert.Equal(t, FlatDiags{
 		{
-			Err:      "error test: 123",
 			Resource: "a",
 			Type:     RESOLVING,
 			Severity: ERROR,
@@ -190,7 +182,6 @@ func TestDiagnostics_SquashRedactable(t *testing.T) {
 
 	assert.Equal(t, FlatDiags{
 		{
-			Err:      "error test: xxx",
 			Resource: "a",
 			Type:     RESOLVING,
 			Severity: ERROR,
@@ -277,7 +268,7 @@ func TestDiagnostics_BySeverity(t *testing.T) {
 			assert.Equal(t, len(tc.expectedErrs), len(res))
 			resErrs := make([]string, len(res))
 			for i := range res {
-				resErrs[i] = res[i].Error()
+				resErrs[i] = res[i].Description().Summary
 			}
 			assert.Equal(t, tc.expectedErrs, resErrs)
 		})

provider/diag/error.go

@@ -54,14 +54,13 @@ func WrapError(err error) error {
 
 // NewBaseError creates a BaseError from given error, except the given error is a BaseError itself
 func NewBaseError(err error, dt Type, opts ...BaseErrorOption) *BaseError {
-	be, ok := err.(*BaseError)
-	if !ok {
-		be = &BaseError{
-			err:            err,
-			diagnosticType: dt,
-			severity:       ERROR,
-		}
+
+	be := &BaseError{
+		err:            err,
+		diagnosticType: dt,
+		severity:       ERROR,
 	}
+
 	for _, o := range opts {
 		o(be)
 	}
@@ -75,12 +74,19 @@ func (e BaseError) Severity() Severity {
 }
 
 func (e BaseError) Description() Description {
-	summary := e.summary
+	summary := ""
+
 	if e.summary == "" {
-		summary = e.Error()
-	} else if e.err != nil {
-		if es := e.err.Error(); es != summary {
-			summary += ": " + es
+		if e.err == nil {
+			summary = "no summary"
+		} else {
+			summary = e.err.Error()
+		}
+	} else {
+		if e.err == nil {
+			summary = e.summary
+		} else {
+			summary = e.summary + ": " + e.err.Error()
 		}
 	}
 
@@ -96,17 +102,6 @@ func (e BaseError) Type() Type {
 	return e.diagnosticType
 }
 
-func (e BaseError) Error() string {
-	// return original error
-	if e.err != nil {
-		return e.err.Error()
-	}
-	if e.summary == "" {
-		return "No summary"
-	}
-	return e.summary
-}
-
 func (e BaseError) Unwrap() error {
 	return e.err
 }
@@ -192,12 +187,5 @@ func FromError(err error, dt Type, opts ...BaseErrorOption) Diagnostics {
 		return nil
 	}
 
-	switch d := err.(type) {
-	case Diagnostics:
-		return d
-	case Diagnostic:
-		return Diagnostics{d}
-	default:
-		return Diagnostics{NewBaseError(err, dt, opts...)}
-	}
+	return Diagnostics{NewBaseError(err, dt, opts...)}
 }

provider/diag/flat.go

@@ -2,9 +2,8 @@ package diag
 
 import "sort"
 
-// FlatDiag is a structured diagnostic, usually can be used to create a json of diagnostics or testing.
+// FlatDiag is a structured diagnostic, used only for tests.
 type FlatDiag struct {
-	Err         string
 	Resource    string
 	ResourceID  []string
 	Type        Type
@@ -17,13 +16,13 @@ type FlatDiags []FlatDiag
 
 var _ sort.Interface = (*FlatDiags)(nil)
 
-// FlattenDiags converts Diagnostics to an array of FlatDiag
+// FlattenDiags converts Diagnostics to an array of FlatDiag.
+// Used only in tests.
 func FlattenDiags(dd Diagnostics, skipDescription bool) FlatDiags {
 	df := make(FlatDiags, len(dd))
 	for i, d := range dd {
 		description := d.Description()
 		df[i] = FlatDiag{
-			Err:      d.Error(),
 			Resource: description.Resource,
 			Type:     d.Type(),
 			Severity: d.Severity(),

provider/diag/native.go

@@ -21,7 +21,3 @@ func (n nativeError) Description() Description {
 func (n nativeError) Err() error {
 	return n.err
 }
-
-func (n nativeError) Error() string {
-	return n.err.Error()
-}

provider/execution/errors.go

@@ -12,21 +12,7 @@ const (
 	fdLimitMessage = "try increasing number of available file descriptors via `ulimit -n 10240` or by increasing timeout via provider specific parameters"
 )
 
-type ErrorClassifier func(meta schema.ClientMeta, resourceName string, err error) diag.Diagnostics
-
-func defaultErrorClassifier(_ schema.ClientMeta, resourceName string, err error) diag.Diagnostics {
-	if _, ok := err.(diag.Diagnostic); ok {
-		return nil
-	}
-	if _, ok := err.(diag.Diagnostics); ok {
-		return nil
-	}
-	if strings.Contains(err.Error(), ": socket: too many open files") {
-		// Return a Diagnostic error so that it can be properly propagated back to the user via the CLI
-		return fromError(err, diag.WithResourceName(resourceName), diag.WithSummary(fdLimitMessage), diag.WithType(diag.THROTTLE), diag.WithSeverity(diag.WARNING))
-	}
-	return nil
-}
+type ErrorClassifier func(meta schema.ClientMeta, resourceName string, err error, summary string, resourcePrimaryKeys []string) diag.Diagnostics
 
 func ClassifyError(err error, opts ...diag.BaseErrorOption) diag.Diagnostics {
 	if err != nil && strings.Contains(err.Error(), ": socket: too many open files") {
@@ -45,15 +31,6 @@ func WithResource(resource *schema.Resource) diag.BaseErrorOption {
 
 func fromError(err error, opts ...diag.BaseErrorOption) diag.Diagnostics {
 	baseOpts := append([]diag.BaseErrorOption{diag.WithNoOverwrite()}, opts...)
-	switch ti := err.(type) {
-	case diag.Diagnostics:
-		ret := make(diag.Diagnostics, 0, len(ti))
-		for _, d := range ti {
-			ret = append(ret, diag.NewBaseError(d, diag.RESOLVING, baseOpts...))
-		}
-		return ret
-	default:
-		e := diag.NewBaseError(err, diag.RESOLVING, baseOpts...)
-		return diag.Diagnostics{e}
-	}
+
+	return diag.Diagnostics{diag.NewBaseError(err, diag.RESOLVING, baseOpts...)}
 }