Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

function: Replace usage of diagnostics with function errors during execution of provider-defined functions #925

Merged
merged 27 commits into from
Feb 28, 2024
Merged

function: Replace usage of diagnostics with function errors during execution of provider-defined functions #925

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
03600f8
Replacing function.RunResponse diagnostics with error
bendbennett Feb 8, 2024
4eff01d
Adding custom FunctionError
bendbennett Feb 9, 2024
ad94609
Adding custom FunctionErrors
bendbennett Feb 12, 2024
6df74dc
Removing unneeded equateErrors gocmp option
bendbennett Feb 12, 2024
f32bdb1
Switching to using convenience functions for adding errors to Functio…
bendbennett Feb 12, 2024
06bd2e2
Add copyright headers
bendbennett Feb 12, 2024
3bf7f79
Refactor to use Error() method on function errors when converting to …
bendbennett Feb 13, 2024
8957a63
Adding documentation and testing for fwerrors types
bendbennett Feb 19, 2024
c14bf43
Formatting errors during conversion to tfprotov<5|6>.FunctionError
bendbennett Feb 20, 2024
b608411
Removing argument error and argument warning diagnostics
bendbennett Feb 20, 2024
e3ab1d2
Renaming field name for FunctionErrors from Error to Errors
bendbennett Feb 20, 2024
7f0c6ee
Modifying documentation to reflect that executing the Run() method of…
bendbennett Feb 20, 2024
f82289c
Remove references to AddArgumentError and AddArgumentWarning from dia…
bendbennett Feb 20, 2024
a488bb8
Removing fwerror package and moving FunctionError to function package
bendbennett Feb 21, 2024
f0d758b
Refactoring to replace FunctionErrors slice with single FunctionError
bendbennett Feb 22, 2024
1419cd2
Bumping terraform-plugin-go to v0.22.0
bendbennett Feb 23, 2024
9bb896b
Removing unneeded DiagnosticWithFunctionArgument interface and implem…
bendbennett Feb 23, 2024
419f3e1
Merge remote-tracking branch 'origin/main' into pdf-err-handling
bendbennett Feb 23, 2024
cb45ef3
Altering function signature of ConcatFuncErrors
bendbennett Feb 26, 2024
18c5015
Removing HasError method
bendbennett Feb 26, 2024
aa853c3
Updating docs
bendbennett Feb 27, 2024
605ae15
Updates following code review
bendbennett Feb 27, 2024
9ea959e
Adding changelog entries
bendbennett Feb 27, 2024
9fb8b29
Fix naming
bendbennett Feb 27, 2024
5ace625
Update website/docs/plugin/framework/functions/errors.mdx
bendbennett Feb 27, 2024
bba24ad
Formatting
bendbennett Feb 27, 2024
c705415
Updates following code review
bendbennett Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions diag/argument_error_diagnostic.go
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions diag/argument_warning_diagnostic.go
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions diag/diagnostic.go
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment placement not really relevant, but I wanted a thread 😆

Open question: Do we think it'd be valuable to note in the diag package doc that this package does not contain function specific error handling? Currently it's kind of generic about the error handling it can do:

terraform-plugin-framework/diag/doc.go

Lines 4 to 8 in 3c7a391

// Package diag implements diagnostic functionality, which is a practitioner
// feedback mechanism for providers. It is designed for display in Terraform
// user interfaces, rather than logging based feedback, which is generally
// saved to a file for later inspection and troubleshooting.
package diag

Maybe we could redirect them to the function package / specific error struct (since this is the only exception currently)

Same question for the package doc for function, should we note that it also contains error handling specific to function implementations:

terraform-plugin-framework/function/doc.go

Lines 4 to 18 in 3c7a391

// Package function contains all interfaces, request types, and response
// types for a Terraform Provider function implementation.
//
// In Terraform, a function is a concept which enables provider developers
// to offer practitioners a pure function call in their configuration. Functions
// are defined by a function name, such as "parse_xyz", a definition
// representing the ordered list of parameters with associated data types and
// a result data type, and the function logic.
//
// The main starting point for implementations in this package is the
// [Function] type which represents an instance of a function that has its own
// argument data when called. The [Function] implementations are referenced by a
// [provider.Provider] type Functions method, which enables the function for
// practitioner and testing usage.
package function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a note into doc.go for both diag and function packages highlighting the need to use FuncError to notify practitioners of issues arising during execution of provider-defined functions.

Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,6 @@ type Diagnostic interface {
Equal(Diagnostic) bool
}

// DiagnosticWithFunctionArgument is a diagnostic associated with a
// function argument.
//
// This information is used to display contextual source configuration to
// practitioners.
type DiagnosticWithFunctionArgument interface {
Diagnostic

// FunctionArgument points to a specific function argument position.
//
// If present, this enables the display of source configuration context for
// supporting implementations such as Terraform CLI commands.
FunctionArgument() int
}

// DiagnosticWithPath is a diagnostic associated with an attribute path.
//
// This attribute information is used to display contextual source configuration
Expand Down
12 changes: 0 additions & 12 deletions diag/diagnostics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,6 @@ import (
// or consistent.
type Diagnostics []Diagnostic

// AddArgumentError adds a generic function argument error diagnostic to the
// collection.
func (diags *Diagnostics) AddArgumentError(position int, summary string, detail string) {
diags.Append(NewArgumentErrorDiagnostic(position, summary, detail))
}

// AddArgumentWarning adds a function argument warning diagnostic to the
// collection.
func (diags *Diagnostics) AddArgumentWarning(position int, summary string, detail string) {
diags.Append(NewArgumentWarningDiagnostic(position, summary, detail))
}

// AddAttributeError adds a generic attribute error diagnostic to the collection.
func (diags *Diagnostics) AddAttributeError(path path.Path, summary string, detail string) {
diags.Append(NewAttributeErrorDiagnostic(path, summary, detail))
Expand Down
124 changes: 0 additions & 124 deletions diag/diagnostics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,130 +12,6 @@ import (
"github.com/hashicorp/terraform-plugin-framework/path"
)

func TestDiagnosticsAddArgumentError(t *testing.T) {
t.Parallel()

testCases := map[string]struct {
diags diag.Diagnostics
position int
summary string
detail string
expected diag.Diagnostics
}{
"nil-add": {
diags: nil,
position: 0,
summary: "one summary",
detail: "one detail",
expected: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
},
},
"add": {
diags: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
},
position: 0,
summary: "three summary",
detail: "three detail",
expected: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
diag.NewArgumentErrorDiagnostic(0, "three summary", "three detail"),
},
},
"duplicate": {
diags: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
},
position: 0,
summary: "one summary",
detail: "one detail",
expected: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
},
},
}

for name, tc := range testCases {
name, tc := name, tc
t.Run(name, func(t *testing.T) {
t.Parallel()

tc.diags.AddArgumentError(tc.position, tc.summary, tc.detail)

if diff := cmp.Diff(tc.diags, tc.expected); diff != "" {
t.Errorf("Unexpected response (+wanted, -got): %s", diff)
}
})
}
}

func TestDiagnosticsAddArgumentWarning(t *testing.T) {
t.Parallel()

testCases := map[string]struct {
diags diag.Diagnostics
position int
summary string
detail string
expected diag.Diagnostics
}{
"nil-add": {
diags: nil,
position: 0,
summary: "one summary",
detail: "one detail",
expected: diag.Diagnostics{
diag.NewArgumentWarningDiagnostic(0, "one summary", "one detail"),
},
},
"add": {
diags: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
},
position: 0,
summary: "three summary",
detail: "three detail",
expected: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
diag.NewArgumentWarningDiagnostic(0, "three summary", "three detail"),
},
},
"duplicate": {
diags: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
},
position: 0,
summary: "two summary",
detail: "two detail",
expected: diag.Diagnostics{
diag.NewArgumentErrorDiagnostic(0, "one summary", "one detail"),
diag.NewArgumentWarningDiagnostic(0, "two summary", "two detail"),
},
},
}

for name, tc := range testCases {
name, tc := name, tc
t.Run(name, func(t *testing.T) {
t.Parallel()

tc.diags.AddArgumentWarning(tc.position, tc.summary, tc.detail)

if diff := cmp.Diff(tc.diags, tc.expected); diff != "" {
t.Errorf("Unexpected response (+wanted, -got): %s", diff)
}
})
}
}

func TestDiagnosticsAddAttributeError(t *testing.T) {
t.Parallel()

Expand Down
54 changes: 0 additions & 54 deletions diag/with_function_argument.go
View file
Open in desktop

This file was deleted.

Loading
Loading