From b2ac1f1871afa6b6e53901022dba49fc076edaa8 Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Thu, 5 Feb 2026 03:04:32 +0000 Subject: [PATCH 1/5] feat: add helpful hints when connection create/upsert fails with invalid resource IDs When using --source-id or --destination-id flags and the API returns a 404 Not Found error, the CLI now displays helpful hints about expected ID formats: - Source IDs typically start with 'src_' - Destination IDs typically start with 'des_' This helps users identify when they've provided the wrong type of ID (e.g., using a connection ID 'web_xxx' instead of a source ID 'src_xxx'). Fixes #204 Example output for the bug report scenario: ``` $ hookdeck connection create --name "my-conn" --source-id "web_y0A7nz0tRxZy" \ --destination-type HTTP --destination-name "my-dest" \ --destination-url "https://example.com" failed to create connection: error: Not Found Hints: - --source-id 'web_y0A7nz0tRxZy' was provided. Source IDs typically start with 'src_'. Please verify the resource IDs exist and are the correct type. ``` Co-authored-by: Cursor --- pkg/cmd/connection_create.go | 33 ++- pkg/cmd/connection_upsert.go | 2 +- .../acceptance/connection_error_hints_test.go | 257 ++++++++++++++++++ 3 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 test/acceptance/connection_error_hints_test.go diff --git a/pkg/cmd/connection_create.go b/pkg/cmd/connection_create.go index 531047a..3172e82 100644 --- a/pkg/cmd/connection_create.go +++ b/pkg/cmd/connection_create.go @@ -522,7 +522,7 @@ func (cc *connectionCreateCmd) runConnectionCreateCmd(cmd *cobra.Command, args [ // Single API call to create the connection connection, err := client.CreateConnection(context.Background(), req) if err != nil { - return fmt.Errorf("failed to create connection: %w", err) + return cc.enhanceCreateError(err) } // Display results @@ -1049,3 +1049,34 @@ func (cc *connectionCreateCmd) buildRulesArray(cmd *cobra.Command) ([]hookdeck.R return rules, nil } + +// enhanceCreateError adds helpful hints to API errors based on the flags used +func (cc *connectionCreateCmd) enhanceCreateError(err error) error { + return cc.enhanceConnectionError(err, "create") +} + +// enhanceConnectionError adds helpful hints to API errors based on the flags used +// This is shared between create and upsert commands +func (cc *connectionCreateCmd) enhanceConnectionError(err error, operation string) error { + errStr := err.Error() + + // Check if this is a "Not Found" error, which commonly indicates an invalid resource ID + isNotFound := strings.Contains(errStr, "Not Found") || strings.Contains(errStr, "404") + + if isNotFound { + var hints []string + + if cc.sourceID != "" { + hints = append(hints, fmt.Sprintf(" - --source-id '%s' was provided. Source IDs typically start with 'src_'.", cc.sourceID)) + } + if cc.destinationID != "" { + hints = append(hints, fmt.Sprintf(" - --destination-id '%s' was provided. Destination IDs typically start with 'des_'.", cc.destinationID)) + } + + if len(hints) > 0 { + return fmt.Errorf("failed to %s connection: %w\n\nHints:\n%s\n\nPlease verify the resource IDs exist and are the correct type.", operation, err, strings.Join(hints, "\n")) + } + } + + return fmt.Errorf("failed to %s connection: %w", operation, err) +} diff --git a/pkg/cmd/connection_upsert.go b/pkg/cmd/connection_upsert.go index 0612c0c..6391edf 100644 --- a/pkg/cmd/connection_upsert.go +++ b/pkg/cmd/connection_upsert.go @@ -368,7 +368,7 @@ func (cu *connectionUpsertCmd) runConnectionUpsertCmd(cmd *cobra.Command, args [ connection, err := client.UpsertConnection(context.Background(), req) if err != nil { - return fmt.Errorf("failed to upsert connection: %w", err) + return cu.enhanceConnectionError(err, "upsert") } // Display results diff --git a/test/acceptance/connection_error_hints_test.go b/test/acceptance/connection_error_hints_test.go new file mode 100644 index 0000000..9ac2966 --- /dev/null +++ b/test/acceptance/connection_error_hints_test.go @@ -0,0 +1,257 @@ +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") + + t.Logf("Successfully verified error hints for non-existent destination ID") +} + +// TestConnectionCreateWithWrongIDType tests error hints when wrong ID type is provided +// This reproduces the bug from issue #204 where a connection ID was passed as source ID +func TestConnectionCreateWithWrongIDType(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + connName := "test-wrong-id-type-" + timestamp + destName := "test-dst-" + timestamp + // Using a connection ID format (web_) instead of source ID format (src_) + wrongIDType := "web_y0A7nz0tRxZy" + + // Try to create connection with wrong ID type + stdout, stderr, err := cli.Run("connection", "create", + "--name", connName, + "--source-id", wrongIDType, + "--destination-name", destName, + "--destination-type", "HTTP", + "--destination-url", "https://example.com/webhooks", + ) + + require.Error(t, err, "Should fail when wrong ID type is provided") + combinedOutput := stdout + stderr + + // Verify error message contains helpful hints about correct ID format + 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, wrongIDType, "Hint should include the provided ID") + assert.Contains(t, combinedOutput, "src_", "Hint should mention correct source ID prefix") + assert.Contains(t, combinedOutput, "verify the resource IDs", "Should suggest verifying resource IDs") + + t.Logf("Successfully verified error hints for wrong ID type (issue #204 scenario)") +} + +// TestConnectionUpsertWithNonExistentSourceID tests error hints for upsert with non-existent source ID +func TestConnectionUpsertWithNonExistentSourceID(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + connName := "test-upsert-bad-src-" + timestamp + destName := "test-dst-" + timestamp + fakeSourceID := "src_nonexistent456" + + // Try to upsert connection with non-existent source ID + stdout, stderr, err := cli.Run("connection", "upsert", 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 upsert connection", "Should indicate connection upsert 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 upsert with non-existent source ID") +} + +// TestConnectionUpsertWithNonExistentDestinationID tests error hints for upsert with non-existent destination ID +func TestConnectionUpsertWithNonExistentDestinationID(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + connName := "test-upsert-bad-dst-" + timestamp + sourceName := "test-src-" + timestamp + fakeDestinationID := "des_nonexistent456" + + // Try to upsert connection with non-existent destination ID + stdout, stderr, err := cli.Run("connection", "upsert", 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 upsert connection", "Should indicate connection upsert 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") + + t.Logf("Successfully verified error hints for upsert with non-existent destination ID") +} + +// TestConnectionCreateWithExistingSourceID tests that create works correctly with a valid existing source ID +// This is a positive test to ensure the --source-id flag works when the source exists +func TestConnectionCreateWithExistingSourceID(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + // First, create a connection to get a source we can reuse + initialConnName := "test-initial-" + timestamp + sourceName := "test-reusable-src-" + timestamp + initialDestName := "test-initial-dst-" + timestamp + + var initialConn Connection + err := cli.RunJSON(&initialConn, + "connection", "create", + "--name", initialConnName, + "--source-name", sourceName, + "--source-type", "WEBHOOK", + "--destination-name", initialDestName, + "--destination-type", "CLI", + "--destination-cli-path", "/initial", + ) + require.NoError(t, err, "Should create initial connection") + require.NotEmpty(t, initialConn.ID, "Initial connection should have ID") + + // Get the source ID from the created connection + sourceID := initialConn.Source.Name // We need the actual source ID, not name + // Actually, let's get the connection details which includes source ID + var connDetails map[string]interface{} + err = cli.RunJSON(&connDetails, "connection", "get", initialConn.ID) + require.NoError(t, err, "Should get connection details") + + source, ok := connDetails["source"].(map[string]interface{}) + require.True(t, ok, "Should have source in connection") + sourceID, ok = source["id"].(string) + require.True(t, ok && sourceID != "", "Should have source ID") + + t.Logf("Created initial connection with source ID: %s", sourceID) + + // Cleanup initial connection + t.Cleanup(func() { + deleteConnection(t, cli, initialConn.ID) + }) + + // Now create a new connection using the existing source ID + newConnName := "test-with-src-id-" + timestamp + newDestName := "test-new-dst-" + timestamp + + var newConn Connection + err = cli.RunJSON(&newConn, + "connection", "create", + "--name", newConnName, + "--source-id", sourceID, + "--destination-name", newDestName, + "--destination-type", "CLI", + "--destination-cli-path", "/new", + ) + require.NoError(t, err, "Should create connection with existing source ID") + require.NotEmpty(t, newConn.ID, "New connection should have ID") + + // Cleanup new connection + t.Cleanup(func() { + deleteConnection(t, cli, newConn.ID) + }) + + // Verify the connection uses the same source + assert.Equal(t, sourceName, newConn.Source.Name, "Should use the existing source") + assert.Equal(t, newDestName, newConn.Destination.Name, "Should have new destination") + + t.Logf("Successfully created connection %s using existing source ID %s", newConn.ID, sourceID) +} From 3039c83d7f53be1a18600164df9b1f242e71ff19 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Feb 2026 11:11:34 +0000 Subject: [PATCH 2/5] Initial plan From 4c70e508dacd0886c46ff9674575814faa78e909 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Feb 2026 11:13:57 +0000 Subject: [PATCH 3/5] fix: correct destination ID prefix from dest_ to des_ in docs and tests Co-authored-by: leggetter <328367+leggetter@users.noreply.github.com> --- REFERENCE.md | 4 ++-- pkg/hookdeck/connections_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 8c1e20d..3da1d94 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -903,7 +903,7 @@ hookdeck connection list hookdeck connection list --source-id src_abc123 # Filter by destination ID -hookdeck connection list --destination-id dest_xyz789 +hookdeck connection list --destination-id des_xyz789 # Filter by connection name hookdeck connection list --name "production-connection" @@ -1586,7 +1586,7 @@ hookdeck attempt list hookdeck attempt list --event-id evt_123 # List attempts for a destination -hookdeck attempt list --destination-id dest_456 +hookdeck attempt list --destination-id des_456 # Filter by status hookdeck attempt list --status FAILED diff --git a/pkg/hookdeck/connections_test.go b/pkg/hookdeck/connections_test.go index b7319bf..8f01963 100644 --- a/pkg/hookdeck/connections_test.go +++ b/pkg/hookdeck/connections_test.go @@ -72,7 +72,7 @@ func TestListConnections(t *testing.T) { "disabled": "false", "paused": "false", "source_id": "src_123", - "destination": "dest_123", + "destination": "des_123", }, mockResponse: ConnectionListResponse{ Models: []Connection{ @@ -267,7 +267,7 @@ func TestCreateConnection(t *testing.T) { Name: stringPtr("test-connection"), Description: stringPtr("test description"), SourceID: stringPtr("src_123"), - DestinationID: stringPtr("dest_123"), + DestinationID: stringPtr("des_123"), }, mockResponse: Connection{ ID: "conn_123", From 40f44de7f0479eaf3b61c102b59e88ea05881fc9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Feb 2026 11:14:30 +0000 Subject: [PATCH 4/5] fix: correct destination ID prefix in README.md example Co-authored-by: leggetter <328367+leggetter@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bde7204..4f183e7 100644 --- a/README.md +++ b/README.md @@ -816,7 +816,7 @@ $ hookdeck connection list # Filter by source or destination $ hookdeck connection list --source src_abc123 -$ hookdeck connection list --destination dest_xyz789 +$ hookdeck connection list --destination des_xyz789 # Filter by name pattern $ hookdeck connection list --name "production-*" From 6db1dc4805161103488fe0f06f45108307c0c978 Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Thu, 5 Feb 2026 11:46:11 +0000 Subject: [PATCH 5/5] fix: remove dead code assignment in acceptance test Remove unnecessary sourceID assignment that was immediately overwritten. Co-authored-by: Cursor --- test/acceptance/connection_error_hints_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/acceptance/connection_error_hints_test.go b/test/acceptance/connection_error_hints_test.go index 9ac2966..761c338 100644 --- a/test/acceptance/connection_error_hints_test.go +++ b/test/acceptance/connection_error_hints_test.go @@ -210,15 +210,13 @@ func TestConnectionCreateWithExistingSourceID(t *testing.T) { require.NotEmpty(t, initialConn.ID, "Initial connection should have ID") // Get the source ID from the created connection - sourceID := initialConn.Source.Name // We need the actual source ID, not name - // Actually, let's get the connection details which includes source ID var connDetails map[string]interface{} err = cli.RunJSON(&connDetails, "connection", "get", initialConn.ID) require.NoError(t, err, "Should get connection details") source, ok := connDetails["source"].(map[string]interface{}) require.True(t, ok, "Should have source in connection") - sourceID, ok = source["id"].(string) + sourceID, ok := source["id"].(string) require.True(t, ok && sourceID != "", "Should have source ID") t.Logf("Created initial connection with source ID: %s", sourceID)