From 0dbe810efcb69f05719226e42ad6c9044b9751bd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 27 Mar 2026 15:15:17 +0000 Subject: [PATCH 1/2] feat: improve onboarding experience with better error messages This commit addresses user feedback about the 'black box' experience when setting up Bruin connections, especially for BigQuery. Key improvements: 1. Service Account File Validation: - Clear error when file is not found with path resolution hints - Explains difference between relative paths (from .bruin.yml) and absolute paths - Detects when JSON content is accidentally used instead of a file path - Validates that the file contains a valid service account key - Step-by-step instructions for creating a new service account key 2. Project ID Validation: - Validates project_id is provided before attempting connection - Clear error message with instructions for finding project ID 3. BigQuery Connection Errors with IAM Hints: - Permission denied errors now suggest specific IAM roles needed - Project not found errors explain possible causes - Invalid credentials errors provide guidance for regenerating keys - ADC (Application Default Credentials) errors provide gcloud commands 4. Improved Connection Test Command: - Shows progress indicator when testing connections - On failure, displays error with formatted hints - Lists available connections when connection not found - Includes connection type in success/failure messages 5. Enhanced Credential Configuration Errors: - Clear message when no credentials are configured - Lists all three credential options with examples These changes particularly help data analysts transitioning to data engineering who may not be familiar with GCP IAM concepts or service account configuration. Co-authored-by: Arsalan --- cmd/connections.go | 75 +++++++++++-- internal/data/embed_darwin_amd64.go | 1 - internal/data/embed_darwin_arm64.go | 1 - internal/data/embed_linux_amd64.go | 1 - internal/data/embed_linux_arm64.go | 1 - internal/data/embed_windows_amd64.go | 1 - pkg/bigquery/db.go | 111 ++++++++++++++++++- pkg/config/connection_errors.go | 37 +++++++ pkg/connection/connection.go | 82 +++++++++++++- pkg/connection/helper.go | 160 +++++++++++++++++++++++++-- pkg/connection/helper_test.go | 29 ++--- 11 files changed, 458 insertions(+), 41 deletions(-) diff --git a/cmd/connections.go b/cmd/connections.go index c716c08b2..b03e72142 100644 --- a/cmd/connections.go +++ b/cmd/connections.go @@ -461,41 +461,87 @@ func PingConnection() *cli.Command { printErrorForOutput(output, errors2.Wrap(err, "failed to select the environment")) return cli.Exit("", 1) } + + if output != "json" { + infoPrinter.Printf("Testing connection '%s' in environment '%s'...\n", name, environment) + } + manager, errs := connection.NewManagerFromConfigWithContext(ctx, cm) if len(errs) > 0 { - // Handle each error in the errs slice + // Check if the error is for the specific connection we're testing for _, err := range errs { - printErrorForOutput(output, errors2.Wrap(err, "failed to create connection manager")) + errStr := err.Error() + if output == "json" { + printConnectionTestErrorJSON(name, errStr) + } else { + errorPrinter.Printf("\nConnection setup failed:\n%s\n", errStr) + } } return cli.Exit("", 1) } conn := manager.GetConnection(name) if conn == nil { - printErrorForOutput(output, &config.MissingConnectionError{ + missingErr := &config.MissingConnectionError{ Name: name, ConfigFilePath: configFilePath, EnvironmentName: cm.SelectedEnvironmentName, - }) + } + if output == "json" { + printConnectionTestErrorJSON(name, missingErr.Error()) + } else { + errorPrinter.Printf("\n%s\n", missingErr.Error()) + fmt.Println() + infoPrinter.Println("Available connections in this environment:") + connTypes := cm.SelectedEnvironment.Connections.ConnectionsSummaryList() + if len(connTypes) == 0 { + fmt.Println(" (none)") + } else { + for connName, connType := range connTypes { + fmt.Printf(" - %s (%s)\n", connName, connType) + } + } + fmt.Println() + infoPrinter.Println("To add a new connection, run: bruin connections add") + } return cli.Exit("", 1) } + // Get connection type for better error messages + connType := manager.GetConnectionType(name) + if tester, ok := conn.(interface { Ping(ctx context.Context) error }); ok { testErr := tester.Ping(ctx) if testErr != nil { - printErrorForOutput(output, errors2.Wrap(testErr, fmt.Sprintf("failed to test connection '%s'", name))) + if output == "json" { + printConnectionTestErrorJSON(name, testErr.Error()) + } else { + errorPrinter.Printf("\nConnection test failed for '%s' (%s):\n", name, connType) + fmt.Printf("\n%s\n", testErr.Error()) + } return cli.Exit("", 1) } } else { - infoPrinter.Printf("Connection '%s' does not support testing yet.\n", name) + if output == "json" { + jsonOutput := map[string]interface{}{ + "status": "skipped", + "message": fmt.Sprintf("Connection '%s' does not support testing yet", name), + } + jsonBytes, _ := json.Marshal(jsonOutput) + fmt.Println(string(jsonBytes)) + } else { + warningPrinter.Printf("Connection '%s' (%s) does not support testing yet.\n", name, connType) + } return nil } if output == "json" { - jsonOutput := map[string]string{ - "status": "success", + jsonOutput := map[string]interface{}{ + "status": "success", + "connection": name, + "type": connType, } jsonBytes, err := json.Marshal(jsonOutput) if err != nil { @@ -504,10 +550,21 @@ func PingConnection() *cli.Command { } fmt.Println(string(jsonBytes)) } else { - infoPrinter.Printf("Successfully tested connection '%s' in environment: %s\n", name, environment) + successPrinter.Printf("\nConnection '%s' (%s) is working correctly!\n", name, connType) } return nil }, } } + +// printConnectionTestErrorJSON outputs a connection test error in JSON format. +func printConnectionTestErrorJSON(name string, errMsg string) { + jsonOutput := map[string]interface{}{ + "status": "error", + "connection": name, + "error": errMsg, + } + jsonBytes, _ := json.Marshal(jsonOutput) + fmt.Println(string(jsonBytes)) +} diff --git a/internal/data/embed_darwin_amd64.go b/internal/data/embed_darwin_amd64.go index 96a875e2d..c465038ca 100644 --- a/internal/data/embed_darwin_amd64.go +++ b/internal/data/embed_darwin_amd64.go @@ -1,4 +1,3 @@ - package data import ( diff --git a/internal/data/embed_darwin_arm64.go b/internal/data/embed_darwin_arm64.go index fc3c42772..5a182d832 100644 --- a/internal/data/embed_darwin_arm64.go +++ b/internal/data/embed_darwin_arm64.go @@ -1,4 +1,3 @@ - package data import ( diff --git a/internal/data/embed_linux_amd64.go b/internal/data/embed_linux_amd64.go index d63209415..8b76786fa 100644 --- a/internal/data/embed_linux_amd64.go +++ b/internal/data/embed_linux_amd64.go @@ -1,4 +1,3 @@ - package data import ( diff --git a/internal/data/embed_linux_arm64.go b/internal/data/embed_linux_arm64.go index 81ff86187..e1b0cd72f 100644 --- a/internal/data/embed_linux_arm64.go +++ b/internal/data/embed_linux_arm64.go @@ -1,4 +1,3 @@ - package data import ( diff --git a/internal/data/embed_windows_amd64.go b/internal/data/embed_windows_amd64.go index f1113eaf2..f93fdf249 100644 --- a/internal/data/embed_windows_amd64.go +++ b/internal/data/embed_windows_amd64.go @@ -1,4 +1,3 @@ - package data import ( diff --git a/pkg/bigquery/db.go b/pkg/bigquery/db.go index ced71b17f..14708a9cb 100644 --- a/pkg/bigquery/db.go +++ b/pkg/bigquery/db.go @@ -535,10 +535,117 @@ func (d *Client) Ping(ctx context.Context) error { // Use the existing RunQueryWithoutResult method err := d.RunQueryWithoutResult(ctx, &q) if err != nil { - return errors.Wrap(err, "failed to run test query on Bigquery connection") + return wrapPingError(err, d.config.ProjectID) } - return nil // Return nil if the query runs successfully + return nil +} + +// BigQueryConnectionError provides detailed error information for BigQuery connection issues. +type BigQueryConnectionError struct { + ProjectID string + Issue string + Hint string + OriginalErr error +} + +func (e *BigQueryConnectionError) Error() string { + var result string + result = e.Issue + + if e.Hint != "" { + result += "\n\nHint: " + e.Hint + } + + return result +} + +func (e *BigQueryConnectionError) Unwrap() error { + return e.OriginalErr +} + +// wrapPingError wraps ping errors with helpful hints based on common error patterns. +func wrapPingError(err error, projectID string) error { + errStr := err.Error() + + // Check for permission denied errors + if strings.Contains(errStr, "Access Denied") || strings.Contains(errStr, "403") || strings.Contains(errStr, "permission") { + return &BigQueryConnectionError{ + ProjectID: projectID, + Issue: "Permission denied when connecting to BigQuery", + Hint: `Your service account may be missing required IAM roles. Required roles: + - BigQuery Data Viewer (roles/bigquery.dataViewer) - to read data + - BigQuery Job User (roles/bigquery.jobUser) - to run queries + - BigQuery Data Editor (roles/bigquery.dataEditor) - to write data (if needed) + +To add roles in Google Cloud Console: + 1. Go to IAM & Admin > IAM + 2. Find your service account email + 3. Click Edit (pencil icon) and add the required roles + +Alternatively, grant 'BigQuery Admin' (roles/bigquery.admin) for full access.`, + OriginalErr: err, + } + } + + // Check for project not found errors + if strings.Contains(errStr, "notFound") || strings.Contains(errStr, "404") || strings.Contains(errStr, "project") && strings.Contains(errStr, "not found") { + return &BigQueryConnectionError{ + ProjectID: projectID, + Issue: fmt.Sprintf("Project '%s' not found or not accessible", projectID), + Hint: `Check that: + 1. The project_id in .bruin.yml matches your Google Cloud project ID exactly + 2. The BigQuery API is enabled in your project (APIs & Services > Enable APIs) + 3. Your service account has access to this project`, + OriginalErr: err, + } + } + + // Check for invalid credentials + if strings.Contains(errStr, "invalid_grant") || strings.Contains(errStr, "Invalid JWT") || strings.Contains(errStr, "credentials") { + return &BigQueryConnectionError{ + ProjectID: projectID, + Issue: "Invalid or expired credentials", + Hint: `Your service account key may be invalid or expired. Try: + 1. Generate a new service account key in Google Cloud Console + (IAM & Admin > Service Accounts > Keys > Add Key > Create new key > JSON) + 2. Update the service_account_file path in .bruin.yml + 3. Re-run 'bruin connections test' to verify`, + OriginalErr: err, + } + } + + // Check for ADC not configured + if strings.Contains(errStr, "could not find default credentials") || strings.Contains(errStr, "ADC") { + return &BigQueryConnectionError{ + ProjectID: projectID, + Issue: "Application Default Credentials not found", + Hint: `Run the following command to set up Application Default Credentials: + gcloud auth application-default login + +Or provide explicit credentials by adding service_account_file to your connection in .bruin.yml`, + OriginalErr: err, + } + } + + // Check for billing not enabled + if strings.Contains(errStr, "billing") || strings.Contains(errStr, "Billing") { + return &BigQueryConnectionError{ + ProjectID: projectID, + Issue: "Billing may not be enabled for this project", + Hint: `BigQuery requires billing to be enabled. Go to: + Google Cloud Console > Billing > Link a billing account to your project`, + OriginalErr: err, + } + } + + // Generic fallback with helpful context + return &BigQueryConnectionError{ + ProjectID: projectID, + Issue: fmt.Sprintf("Failed to connect to BigQuery: %s", errStr), + Hint: "Run 'bruin connections test --name ' to debug connection issues.", + OriginalErr: err, + } } func (d *Client) IsPartitioningOrClusteringMismatch(ctx context.Context, meta *bigquery.TableMetadata, asset *pipeline.Asset) bool { diff --git a/pkg/config/connection_errors.go b/pkg/config/connection_errors.go index f8bd71bbf..f255ecbff 100644 --- a/pkg/config/connection_errors.go +++ b/pkg/config/connection_errors.go @@ -68,3 +68,40 @@ func (e *MissingConnectionError) Error() string { environmentName, ) } + +// ConnectionSetupError provides detailed error information for connection setup issues. +type ConnectionSetupError struct { + ConnectionName string + ConnectionType string + Issue string + Hint string + OriginalErr error +} + +func (e *ConnectionSetupError) Error() string { + var sb strings.Builder + + if e.ConnectionType != "" { + sb.WriteString(e.ConnectionType) + sb.WriteString(" ") + } + sb.WriteString("connection") + if e.ConnectionName != "" { + sb.WriteString(" '") + sb.WriteString(e.ConnectionName) + sb.WriteString("'") + } + sb.WriteString(": ") + sb.WriteString(e.Issue) + + if e.Hint != "" { + sb.WriteString("\n\nHint: ") + sb.WriteString(e.Hint) + } + + return sb.String() +} + +func (e *ConnectionSetupError) Unwrap() error { + return e.OriginalErr +} diff --git a/pkg/connection/connection.go b/pkg/connection/connection.go index 4f6c4bf52..aa3a3911c 100644 --- a/pkg/connection/connection.go +++ b/pkg/connection/connection.go @@ -270,11 +270,27 @@ func (m *Manager) AddBqConnectionFromConfig(connection *config.GoogleCloudPlatfo } m.mutex.Unlock() + // Validate project_id is provided + if strings.TrimSpace(connection.ProjectID) == "" { + return &BigQueryConfigError{ + ConnectionName: connection.Name, + Issue: "missing required field 'project_id'", + Hint: "Add 'project_id' to your google_cloud_platform connection in .bruin.yml.\nYou can find your project ID in Google Cloud Console at the top of the page.", + } + } + // Check if we have valid credentials configuration hasExplicitCredentials := len(connection.ServiceAccountFile) > 0 || len(connection.ServiceAccountJSON) > 0 || connection.GetCredentials() != nil if !hasExplicitCredentials && !connection.UseApplicationDefaultCredentials { - return errors.New("credentials are required: provide either service_account_file, service_account_json, or enable use_application_default_credentials") + return &BigQueryConfigError{ + ConnectionName: connection.Name, + Issue: "no credentials configured", + Hint: `Provide credentials using one of these methods: + 1. service_account_file: Path to a JSON key file (relative to .bruin.yml or absolute) + 2. service_account_json: JSON content of a service account key + 3. use_application_default_credentials: true (uses 'gcloud auth application-default login')`, + } } // Validate ServiceAccountFile if provided. @@ -302,7 +318,7 @@ func (m *Manager) AddBqConnectionFromConfig(connection *config.GoogleCloudPlatfo UseApplicationDefaultCredentials: connection.UseApplicationDefaultCredentials, }) if err != nil { - return err + return wrapBigQueryClientError(err, connection.Name) } // Lock and store the new BigQuery client. @@ -315,6 +331,68 @@ func (m *Manager) AddBqConnectionFromConfig(connection *config.GoogleCloudPlatfo return nil } +// BigQueryConfigError provides detailed error information for BigQuery configuration issues. +type BigQueryConfigError struct { + ConnectionName string + Issue string + Hint string + OriginalErr error +} + +func (e *BigQueryConfigError) Error() string { + var sb strings.Builder + if e.ConnectionName != "" { + sb.WriteString("BigQuery connection '") + sb.WriteString(e.ConnectionName) + sb.WriteString("': ") + } + sb.WriteString(e.Issue) + + if e.Hint != "" { + sb.WriteString("\n\n") + sb.WriteString("Hint: ") + sb.WriteString(e.Hint) + } + + return sb.String() +} + +func (e *BigQueryConfigError) Unwrap() error { + return e.OriginalErr +} + +// wrapBigQueryClientError wraps BigQuery client creation errors with helpful hints. +func wrapBigQueryClientError(err error, connectionName string) error { + errStr := err.Error() + + // Check for common error patterns and provide helpful hints + if strings.Contains(errStr, "no credentials provided") { + return &BigQueryConfigError{ + ConnectionName: connectionName, + Issue: "no credentials provided", + Hint: `Provide credentials using one of these methods: + 1. service_account_file: Path to a JSON key file + 2. service_account_json: JSON content of a service account key + 3. use_application_default_credentials: true`, + OriginalErr: err, + } + } + + if strings.Contains(errStr, "could not find default credentials") { + return &BigQueryConfigError{ + ConnectionName: connectionName, + Issue: "Application Default Credentials not found", + Hint: `To set up Application Default Credentials, run: + gcloud auth application-default login + +Or provide explicit credentials using service_account_file or service_account_json in .bruin.yml`, + OriginalErr: err, + } + } + + return errors.Wrapf(err, "failed to create BigQuery connection '%s'", connectionName) +} + func (m *Manager) AddSfConnectionFromConfig(connection *config.SnowflakeConnection) error { m.mutex.Lock() if m.Snowflake == nil { diff --git a/pkg/connection/helper.go b/pkg/connection/helper.go index eb6b2a51f..18b37e7ce 100644 --- a/pkg/connection/helper.go +++ b/pkg/connection/helper.go @@ -3,38 +3,178 @@ package connection import ( "encoding/json" "os" - - "github.com/pkg/errors" + "path/filepath" + "strings" ) -// New helper function to validate ServiceAccountFile. +// ServiceAccountFileError provides detailed error information for service account file issues. +type ServiceAccountFileError struct { + FilePath string + Issue string + Hint string + OriginalErr error +} + +func (e *ServiceAccountFileError) Error() string { + var sb strings.Builder + sb.WriteString(e.Issue) + + if e.Hint != "" { + sb.WriteString("\n\n") + sb.WriteString("Hint: ") + sb.WriteString(e.Hint) + } + + return sb.String() +} + +func (e *ServiceAccountFileError) Unwrap() error { + return e.OriginalErr +} + +// validateServiceAccountFile validates the service account file path and contents. func validateServiceAccountFile(filePath string) error { + // Check if the path looks like JSON content instead of a file path var jsonStr json.RawMessage if err := json.Unmarshal([]byte(filePath), &jsonStr); err == nil { - return errors.New("please use service_account_json instead of service_account_file to define json") + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "the 'service_account_file' field contains JSON content instead of a file path", + Hint: "Use 'service_account_json' to provide JSON credentials directly, or use 'service_account_file' with a path to a credentials file.", + } } + // Check if the file exists + fileInfo, err := os.Stat(filePath) + if os.IsNotExist(err) { + absPath, _ := filepath.Abs(filePath) + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "service account file not found at '" + filePath + "'", + Hint: formatServiceAccountFileHint(filePath, absPath), + OriginalErr: err, + } + } + if err != nil { + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "cannot access service account file at '" + filePath + "': " + err.Error(), + Hint: "Check that the file exists and you have read permissions.", + OriginalErr: err, + } + } + + // Check if it's actually a file (not a directory) + if fileInfo.IsDir() { + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "'" + filePath + "' is a directory, not a file", + Hint: "Provide the path to the service account JSON file, e.g., 'credentials/service-account.json'.", + } + } + + // Read and validate the file contents file, err := os.ReadFile(filePath) if err != nil { - return errors.Errorf("failed to read service account file at '%s': %v", filePath, err) + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "failed to read service account file at '" + filePath + "': " + err.Error(), + Hint: "Check that the file has proper read permissions.", + OriginalErr: err, + } } + + // Validate that the file contains valid JSON var js json.RawMessage if err := json.Unmarshal(file, &js); err != nil { - return errors.Errorf("invalid JSON format in service account file at '%s'", filePath) + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "invalid JSON format in service account file at '" + filePath + "'", + Hint: "Ensure the file contains valid JSON. You can download a new service account key from Google Cloud Console: IAM & Admin > Service Accounts > Keys > Add Key > Create new key > JSON.", + OriginalErr: err, + } + } + + // Validate that it looks like a service account key + var keyData map[string]interface{} + if err := json.Unmarshal(file, &keyData); err == nil { + if _, hasType := keyData["type"]; !hasType { + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "the file at '" + filePath + "' does not appear to be a valid service account key (missing 'type' field)", + Hint: "Download a new service account key from Google Cloud Console: IAM & Admin > Service Accounts > Keys > Add Key > Create new key > JSON.", + } + } + if keyType, ok := keyData["type"].(string); ok && keyType != "service_account" { + return &ServiceAccountFileError{ + FilePath: filePath, + Issue: "the file at '" + filePath + "' has type '" + keyType + "' instead of 'service_account'", + Hint: "Ensure you're using a service account key file, not an OAuth client or other credential type.", + } + } } + return nil } -// New helper function to validate ServiceAccountJSON. +// formatServiceAccountFileHint provides helpful guidance for file not found errors. +func formatServiceAccountFileHint(originalPath, absPath string) string { + var hints []string + + // Check if it's a relative path + if !filepath.IsAbs(originalPath) { + hints = append(hints, "The path '"+originalPath+"' is relative to your .bruin.yml file location.") + hints = append(hints, "Resolved absolute path: "+absPath) + hints = append(hints, "You can use either:") + hints = append(hints, " - A relative path from the .bruin.yml file (e.g., 'credentials/service-account.json')") + hints = append(hints, " - An absolute path (e.g., '/home/user/credentials/service-account.json')") + } + + hints = append(hints, "To create a new service account key:") + hints = append(hints, " 1. Go to Google Cloud Console > IAM & Admin > Service Accounts") + hints = append(hints, " 2. Select your service account (or create one)") + hints = append(hints, " 3. Go to Keys > Add Key > Create new key > JSON") + hints = append(hints, " 4. Save the downloaded file and update service_account_file in .bruin.yml") + + return strings.Join(hints, "\n") +} + +// validateServiceAccountJSON validates the service account JSON content. func validateServiceAccountJSON(jsonStr string) error { - // Check if the path exists and is a file + // Check if the path exists and is a file (user probably meant to use service_account_file) if _, err := os.Stat(jsonStr); err == nil { - return errors.New("please use service_account_file instead of service_account_json to define path") + return &ServiceAccountFileError{ + FilePath: jsonStr, + Issue: "the 'service_account_json' field contains a file path instead of JSON content", + Hint: "Use 'service_account_file' to provide a path to a credentials file, or paste the JSON content directly into 'service_account_json'.", + } } var js json.RawMessage if err := json.Unmarshal([]byte(jsonStr), &js); err != nil { - return errors.Errorf("invalid JSON format in service account JSON") + return &ServiceAccountFileError{ + Issue: "invalid JSON format in service_account_json", + Hint: "Ensure the value is valid JSON. If you're providing a file path, use 'service_account_file' instead.", + OriginalErr: err, + } } + + // Validate that it looks like a service account key + var keyData map[string]interface{} + if err := json.Unmarshal([]byte(jsonStr), &keyData); err == nil { + if _, hasType := keyData["type"]; !hasType { + return &ServiceAccountFileError{ + Issue: "service_account_json does not appear to be a valid service account key (missing 'type' field)", + Hint: "Ensure you're using JSON from a service account key file. You can generate one from Google Cloud Console: IAM & Admin > Service Accounts > Keys.", + } + } + if keyType, ok := keyData["type"].(string); ok && keyType != "service_account" { + return &ServiceAccountFileError{ + Issue: "service_account_json has type '" + keyType + "' instead of 'service_account'", + Hint: "Ensure you're using a service account key, not an OAuth client or other credential type.", + } + } + } + return nil } diff --git a/pkg/connection/helper_test.go b/pkg/connection/helper_test.go index 62ffeb768..eb2a8cda6 100644 --- a/pkg/connection/helper_test.go +++ b/pkg/connection/helper_test.go @@ -2,6 +2,7 @@ package connection import ( "os" + "strings" "testing" ) @@ -43,6 +44,8 @@ func TestValidateServiceAccountFile(t *testing.T) { // Test invalid file. if err := validateServiceAccountFile("invalid_path.json"); err == nil { t.Error("expected error for invalid file path, got none") + } else if !strings.Contains(err.Error(), "service account file not found") { + t.Errorf("expected error about file not found, got: %v", err) } // Test empty file. @@ -61,8 +64,8 @@ func TestValidateServiceAccountFile(t *testing.T) { jsonString := `{"type": "service_account"}` if err := validateServiceAccountFile(jsonString); err == nil { t.Error("expected error for valid JSON string as filePath, got none") - } else if err.Error() != "please use service_account_json instead of service_account_file to define json" { - t.Errorf("expected specific error message, got: %v", err) + } else if !strings.Contains(err.Error(), "service_account_file") && !strings.Contains(err.Error(), "JSON content") { + t.Errorf("expected error message about using service_account_json, got: %v", err) } } @@ -92,18 +95,18 @@ func TestValidateServiceAccountJSON(t *testing.T) { invalidJSON := `{"type": "service_account",}` if err := validateServiceAccountJSON(invalidJSON); err == nil { t.Error("expected error for invalid JSON format, got none") - } else if err.Error() != "invalid JSON format in service account JSON" { - t.Errorf("expected specific error message, got: %v", err) + } else if !strings.Contains(err.Error(), "invalid JSON format") { + t.Errorf("expected error about invalid JSON format, got: %v", err) } // Test using a file path that doesn't exist. if err := validateServiceAccountJSON("some_file_path.json"); err == nil { t.Error("expected error for file path, got none") - } else if err.Error() != "invalid JSON format in service account JSON" { - t.Errorf("expected specific error message, got: %v", err) + } else if !strings.Contains(err.Error(), "invalid JSON format") { + t.Errorf("expected error about invalid JSON format, got: %v", err) } - // Test using a file path that exist + // Test using a file path that exists tempFile, err := os.CreateTemp(t.TempDir(), "service_account.json") if err != nil { t.Fatalf("failed to create temp file: %v", err) @@ -117,23 +120,23 @@ func TestValidateServiceAccountJSON(t *testing.T) { if err := validateServiceAccountJSON(tempFile.Name()); err == nil { t.Error("expected error for file path, got none") - } else if err.Error() != "please use service_account_file instead of service_account_json to define path" { - t.Errorf("expected specific error message, got: %v", err) + } else if !strings.Contains(err.Error(), "service_account_json") && !strings.Contains(err.Error(), "file path") { + t.Errorf("expected error about using service_account_file, got: %v", err) } // Test empty JSON string. emptyJSON := `` if err := validateServiceAccountJSON(emptyJSON); err == nil { t.Error("expected error for empty JSON string, got none") - } else if err.Error() != "invalid JSON format in service account JSON" { - t.Errorf("expected specific error message, got: %v", err) + } else if !strings.Contains(err.Error(), "invalid JSON format") { + t.Errorf("expected error about invalid JSON format, got: %v", err) } // Test malformed JSON string. malformedJSON := `{"type": "service_account", "project_id": "bruin-common-health-check", "private_key_id": "TEST", "private_key": "-----BEGIN PRIVATE KEY-----\nTEST\n-----END PRIVATE KEY-----\n", "client_email": "bruin-health-check@bruin-common-health-check.iam.gserviceaccount.com", "client_id": "TEST", "auth_uri": "https://accounts.google.com/o/oauth2/auth", "token_uri": "https://oauth2.googleapis.com/token", "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/bruin-health-check%40bruin-common-health-check.iam.gserviceaccount.com", "universe_domain": "googleapis.com"` if err := validateServiceAccountJSON(malformedJSON); err == nil { t.Error("expected error for malformed JSON string, got none") - } else if err.Error() != "invalid JSON format in service account JSON" { - t.Errorf("expected specific error message, got: %v", err) + } else if !strings.Contains(err.Error(), "invalid JSON format") { + t.Errorf("expected error about invalid JSON format, got: %v", err) } } From 48dd7143fd8fe5322cac794faa03fd351306aefb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 27 Mar 2026 15:30:00 +0000 Subject: [PATCH 2/2] fix: address linting issues - Replace fmt.Sprintf with string concatenation (perfsprint) - Fix unchecked json.Marshal error (errchkjson) Co-authored-by: Arsalan --- cmd/connections.go | 8 ++++++-- pkg/bigquery/db.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/connections.go b/cmd/connections.go index b03e72142..4759f4a69 100644 --- a/cmd/connections.go +++ b/cmd/connections.go @@ -560,11 +560,15 @@ func PingConnection() *cli.Command { // printConnectionTestErrorJSON outputs a connection test error in JSON format. func printConnectionTestErrorJSON(name string, errMsg string) { - jsonOutput := map[string]interface{}{ + jsonOutput := map[string]string{ "status": "error", "connection": name, "error": errMsg, } - jsonBytes, _ := json.Marshal(jsonOutput) + jsonBytes, err := json.Marshal(jsonOutput) + if err != nil { + errorPrinter.Printf("failed to marshal error JSON: %v\n", err) + return + } fmt.Println(string(jsonBytes)) } diff --git a/pkg/bigquery/db.go b/pkg/bigquery/db.go index 14708a9cb..571d4e92b 100644 --- a/pkg/bigquery/db.go +++ b/pkg/bigquery/db.go @@ -642,7 +642,7 @@ Or provide explicit credentials by adding service_account_file to your connectio // Generic fallback with helpful context return &BigQueryConnectionError{ ProjectID: projectID, - Issue: fmt.Sprintf("Failed to connect to BigQuery: %s", errStr), + Issue: "Failed to connect to BigQuery: " + errStr, Hint: "Run 'bruin connections test --name ' to debug connection issues.", OriginalErr: err, }