-
Notifications
You must be signed in to change notification settings - Fork 15
feat: add helpful hints when connection create/upsert fails with invalid resource IDs #205
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
b2ac1f1
3039c83
4c70e50
40f44de
a6291e2
6db1dc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,255 @@ | ||||||
| package acceptance | ||||||
|
|
||||||
| import ( | ||||||
| "testing" | ||||||
|
|
||||||
| "github.com/stretchr/testify/assert" | ||||||
| "github.com/stretchr/testify/require" | ||||||
| ) | ||||||
|
|
||||||
| // TestConnectionCreateWithNonExistentSourceID tests error hints when source ID doesn't exist | ||||||
| func TestConnectionCreateWithNonExistentSourceID(t *testing.T) { | ||||||
| if testing.Short() { | ||||||
| t.Skip("Skipping acceptance test in short mode") | ||||||
| } | ||||||
|
|
||||||
| cli := NewCLIRunner(t) | ||||||
| timestamp := generateTimestamp() | ||||||
|
|
||||||
| connName := "test-bad-src-" + timestamp | ||||||
| destName := "test-dst-" + timestamp | ||||||
| fakeSourceID := "src_nonexistent123" | ||||||
|
|
||||||
| // Try to create connection with non-existent source ID | ||||||
| stdout, stderr, err := cli.Run("connection", "create", | ||||||
| "--name", connName, | ||||||
| "--source-id", fakeSourceID, | ||||||
| "--destination-name", destName, | ||||||
| "--destination-type", "CLI", | ||||||
| "--destination-cli-path", "/webhooks", | ||||||
| ) | ||||||
|
|
||||||
| require.Error(t, err, "Should fail when source ID doesn't exist") | ||||||
| combinedOutput := stdout + stderr | ||||||
|
|
||||||
| // Verify error message contains helpful hints | ||||||
| assert.Contains(t, combinedOutput, "failed to create connection", "Should indicate connection creation failed") | ||||||
| assert.Contains(t, combinedOutput, "Hints:", "Should contain hints section") | ||||||
| assert.Contains(t, combinedOutput, "--source-id", "Hint should mention --source-id flag") | ||||||
| assert.Contains(t, combinedOutput, fakeSourceID, "Hint should include the provided source ID") | ||||||
| assert.Contains(t, combinedOutput, "src_", "Hint should mention source ID prefix format") | ||||||
|
|
||||||
| t.Logf("Successfully verified error hints for non-existent source ID") | ||||||
| } | ||||||
|
|
||||||
| // TestConnectionCreateWithNonExistentDestinationID tests error hints when destination ID doesn't exist | ||||||
| func TestConnectionCreateWithNonExistentDestinationID(t *testing.T) { | ||||||
| if testing.Short() { | ||||||
| t.Skip("Skipping acceptance test in short mode") | ||||||
| } | ||||||
|
|
||||||
| cli := NewCLIRunner(t) | ||||||
| timestamp := generateTimestamp() | ||||||
|
|
||||||
| connName := "test-bad-dst-" + timestamp | ||||||
| sourceName := "test-src-" + timestamp | ||||||
| fakeDestinationID := "des_nonexistent123" | ||||||
|
||||||
|
|
||||||
| // Try to create connection with non-existent destination ID | ||||||
| stdout, stderr, err := cli.Run("connection", "create", | ||||||
| "--name", connName, | ||||||
| "--source-name", sourceName, | ||||||
| "--source-type", "WEBHOOK", | ||||||
| "--destination-id", fakeDestinationID, | ||||||
| ) | ||||||
|
|
||||||
| require.Error(t, err, "Should fail when destination ID doesn't exist") | ||||||
| combinedOutput := stdout + stderr | ||||||
|
|
||||||
| // Verify error message contains helpful hints | ||||||
| assert.Contains(t, combinedOutput, "failed to create connection", "Should indicate connection creation failed") | ||||||
| assert.Contains(t, combinedOutput, "Hints:", "Should contain hints section") | ||||||
| assert.Contains(t, combinedOutput, "--destination-id", "Hint should mention --destination-id flag") | ||||||
| assert.Contains(t, combinedOutput, fakeDestinationID, "Hint should include the provided destination ID") | ||||||
| assert.Contains(t, combinedOutput, "des_", "Hint should mention destination ID prefix format") | ||||||
|
||||||
| assert.Contains(t, combinedOutput, "des_", "Hint should mention destination ID prefix format") | |
| assert.Contains(t, combinedOutput, "dest_", "Hint should mention destination ID prefix format") |
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fake destination ID uses an incorrect prefix. According to the documentation (REFERENCE.md:906) and unit tests (pkg/hookdeck/connections_test.go:270), destination IDs should use the prefix 'dest_', not 'des_'. Change this to "dest_nonexistent456" to match the actual API format.
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test assertion checks for the incorrect prefix 'des_'. According to the documentation (REFERENCE.md:906) and unit tests (pkg/hookdeck/connections_test.go:270), destination IDs should use the prefix 'dest_', not 'des_'. Update this assertion to check for "dest_" instead.
| assert.Contains(t, combinedOutput, "des_", "Hint should mention destination ID prefix format") | |
| assert.Contains(t, combinedOutput, "dest_", "Hint should mention destination ID prefix format") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The destination ID prefix in the hint message is incorrect. According to the documentation (REFERENCE.md:906) and unit tests (pkg/hookdeck/connections_test.go:270), destination IDs use the prefix 'dest_', not 'des_'. The hint message should say "Destination IDs typically start with 'dest_'." to be consistent with the actual API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot you are incorrect. destination IDs start with

des_. Where did you get your information from?