Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [Unreleased]

- Classify `--error-format json` errors from typed errors instead of message substrings, making structured error output reliable when wording changes (#12)
- Add `--timeout` flag and propagate request contexts so commands abort cleanly on Ctrl+C, with clear timeout and cancellation errors (#11)
- Add `MB_SESSION_TOKEN` as an alternative authentication method for users without admin access to mint an API key; mutually exclusive with `MB_API_KEY` (#9)
- Document changelog update workflow in `AGENTS.md` and add a pull request template prompting contributors to update `CHANGELOG.md` for user-visible changes (#20)
Expand Down
17 changes: 11 additions & 6 deletions internal/cli/card.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package cli

import (
"errors"
"fmt"
"net/http"
"os"
"strconv"
"strings"

"github.com/andreagrandi/mb-cli/internal/client"
"github.com/andreagrandi/mb-cli/internal/formatter"
"github.com/andreagrandi/mb-cli/internal/mberr"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -179,12 +182,14 @@ func formatQueryResultOutput(cmd *cobra.Command, result *client.QueryResult) err
}

func wrapParameterizedRunError(err error) error {
message := err.Error()
if strings.Contains(message, "API request failed with status 400") {
return fmt.Errorf("parameterized query failed: check parameter keys and values (%w)", err)
}
if strings.Contains(message, "API request failed with status 404") {
return fmt.Errorf("query target was not found (%w)", err)
var apiErr *mberr.APIError
if errors.As(err, &apiErr) {
switch apiErr.StatusCode {
case http.StatusBadRequest:
return &mberr.ParameterizedQueryError{Err: err}
case http.StatusNotFound:
return fmt.Errorf("query target was not found (%w)", err)
}
}
return err
}
26 changes: 21 additions & 5 deletions internal/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/andreagrandi/mb-cli/internal/client"
"github.com/andreagrandi/mb-cli/internal/formatter"
"github.com/andreagrandi/mb-cli/internal/mberr"
"github.com/andreagrandi/mb-cli/internal/validation"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -237,15 +238,21 @@ func matchTableByName(tables []client.TableMetadata, name string) (int, error) {

switch len(matches) {
case 0:
return 0, fmt.Errorf("no table matching '%s' found", name)
return 0, &mberr.ResolutionError{
Kind: mberr.ResourceTable,
Message: fmt.Sprintf("no table matching '%s' found", name),
}
case 1:
return matches[0].ID, nil
default:
names := make([]string, len(matches))
for i, t := range matches {
names[i] = fmt.Sprintf("%s (id=%d)", t.Name, t.ID)
}
return 0, fmt.Errorf("ambiguous table name '%s', matches: %s. Use table ID instead", name, strings.Join(names, ", "))
return 0, &mberr.ResolutionError{
Kind: mberr.ResourceTable,
Message: fmt.Sprintf("ambiguous table name '%s', matches: %s. Use table ID instead", name, strings.Join(names, ", ")),
}
}
}

Expand All @@ -270,7 +277,10 @@ func resolveFieldID(fields []client.Field, name string) (int, error) {
return f.ID, nil
}
}
return 0, fmt.Errorf("no field matching '%s' found in table", name)
return 0, &mberr.ResolutionError{
Kind: mberr.ResourceField,
Message: fmt.Sprintf("no field matching '%s' found in table", name),
}
}

func matchDatabaseByName(databases []client.Database, name string) (int, error) {
Expand All @@ -284,14 +294,20 @@ func matchDatabaseByName(databases []client.Database, name string) (int, error)

switch len(matches) {
case 0:
return 0, fmt.Errorf("no database matching '%s' found", name)
return 0, &mberr.ResolutionError{
Kind: mberr.ResourceDatabase,
Message: fmt.Sprintf("no database matching '%s' found", name),
}
case 1:
return matches[0].ID, nil
default:
names := make([]string, len(matches))
for i, db := range matches {
names[i] = fmt.Sprintf("%s (id=%d)", db.Name, db.ID)
}
return 0, fmt.Errorf("ambiguous database name '%s', matches: %s. Use database ID instead", name, strings.Join(names, ", "))
return 0, &mberr.ResolutionError{
Kind: mberr.ResourceDatabase,
Message: fmt.Sprintf("ambiguous database name '%s', matches: %s. Use database ID instead", name, strings.Join(names, ", ")),
}
}
}
91 changes: 64 additions & 27 deletions internal/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ package cli
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"os"
"os/signal"
"strings"
"syscall"
"time"

"github.com/andreagrandi/mb-cli/internal/mberr"
"github.com/andreagrandi/mb-cli/internal/version"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -90,39 +92,74 @@ func ClassifyError(err error) (errorType, suggestion string) {
return classifyError(err)
}

// classifyError inspects the error's type (not its message text) to determine
// the structured error type and an actionable suggestion.
func classifyError(err error) (errorType, suggestion string) {
msg := err.Error()

switch {
case strings.Contains(msg, "MB_HOST") || strings.Contains(msg, "MB_API_KEY"):
var configErr *mberr.ConfigError
if errors.As(err, &configErr) {
return "CONFIG_ERROR", "Set MB_HOST and MB_API_KEY environment variables"
case strings.Contains(msg, "request timed out"):
}

var timeoutErr *mberr.TimeoutError
if errors.As(err, &timeoutErr) {
return "TIMEOUT_ERROR", "Increase --timeout or check connectivity to MB_HOST"
case strings.Contains(msg, "request canceled"):
}

var canceledErr *mberr.CanceledError
if errors.As(err, &canceledErr) {
return "CANCELED_ERROR", ""
case strings.Contains(msg, "parameterized query failed"):
}

var paramErr *mberr.ParameterizedQueryError
if errors.As(err, &paramErr) {
return "API_ERROR", "Check parameter IDs with 'mb-cli dashboard get <id>' or 'mb-cli card get <id> --full'"
case strings.Contains(msg, "API request failed with status 401"),
strings.Contains(msg, "API request failed with status 403"):
return "AUTH_ERROR", "Check that MB_API_KEY is valid and can access the requested resource"
case strings.Contains(msg, "failed to get dashboard") && strings.Contains(msg, "status 404"):
return "API_ERROR", "Check that the dashboard ID exists and is visible to this API key"
case strings.Contains(msg, "failed to get card") && strings.Contains(msg, "status 404"):
return "API_ERROR", "Check that the card ID exists and is visible to this API key"
case strings.Contains(msg, "failed to get values for dashboard") && strings.Contains(msg, "status 404"):
return "API_ERROR", "Check that the dashboard parameter ID exists for this dashboard"
case strings.Contains(msg, "API request failed with status"):
}

var resolutionErr *mberr.ResolutionError
if errors.As(err, &resolutionErr) {
return "RESOLUTION_ERROR", resolutionSuggestion(resolutionErr.Kind)
}

var apiErr *mberr.APIError
if errors.As(err, &apiErr) {
if apiErr.IsAuth() {
return "AUTH_ERROR", "Check that MB_API_KEY is valid and can access the requested resource"
}
var reqErr *mberr.RequestError
if apiErr.StatusCode == http.StatusNotFound && errors.As(err, &reqErr) {
return "API_ERROR", notFoundSuggestion(reqErr.Resource)
}
return "API_ERROR", ""
case strings.Contains(msg, "no database matching"),
strings.Contains(msg, "ambiguous database name"):
return "RESOLUTION_ERROR", "Use a database ID instead of a name"
case strings.Contains(msg, "no table matching"),
strings.Contains(msg, "ambiguous table name"):
return "RESOLUTION_ERROR", "Use a table ID instead of a name"
case strings.Contains(msg, "no field matching"):
return "RESOLUTION_ERROR", "Check field names with 'mb-cli table metadata <id>'"
}

return "GENERAL_ERROR", ""
}

// resolutionSuggestion returns advice for a name-to-ID resolution failure.
func resolutionSuggestion(kind mberr.ResourceKind) string {
switch kind {
case mberr.ResourceDatabase:
return "Use a database ID instead of a name"
case mberr.ResourceTable:
return "Use a table ID instead of a name"
case mberr.ResourceField:
return "Check field names with 'mb-cli table metadata <id>'"
default:
return ""
}
}

// notFoundSuggestion returns advice for a 404 on a specific resource request.
func notFoundSuggestion(kind mberr.ResourceKind) string {
switch kind {
case mberr.ResourceDashboard:
return "Check that the dashboard ID exists and is visible to this API key"
case mberr.ResourceCard:
return "Check that the card ID exists and is visible to this API key"
case mberr.ResourceDashboardParameter:
return "Check that the dashboard parameter ID exists for this dashboard"
default:
return "GENERAL_ERROR", ""
return ""
}
}

Expand Down
8 changes: 7 additions & 1 deletion internal/client/cards.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"net/http"
"net/url"

"github.com/andreagrandi/mb-cli/internal/mberr"
)

// ListCards retrieves all saved questions (cards).
Expand All @@ -29,7 +31,11 @@ func (c *Client) GetCard(ctx context.Context, id int) (*Card, error) {

resp, err := c.Get(ctx, fmt.Sprintf("/api/card/%d", id), params)
if err != nil {
return nil, fmt.Errorf("failed to get card %d: %w", id, err)
return nil, &mberr.RequestError{
Resource: mberr.ResourceCard,
Op: fmt.Sprintf("failed to get card %d", id),
Err: err,
}
}

var card Card
Expand Down
7 changes: 4 additions & 3 deletions internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"os"

"github.com/andreagrandi/mb-cli/internal/config"
"github.com/andreagrandi/mb-cli/internal/mberr"
"github.com/andreagrandi/mb-cli/internal/version"
)

Expand Down Expand Up @@ -69,7 +70,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
if resp.StatusCode >= 400 {
defer resp.Body.Close()
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("API request failed with status %d: %s", resp.StatusCode, string(body))
return nil, &mberr.APIError{StatusCode: resp.StatusCode, Body: string(body)}
}

return resp, nil
Expand All @@ -80,9 +81,9 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
func requestError(ctx context.Context, err error) error {
switch {
case errors.Is(err, context.DeadlineExceeded), ctx.Err() == context.DeadlineExceeded:
return fmt.Errorf("request timed out: %w", err)
return &mberr.TimeoutError{Err: err}
case errors.Is(err, context.Canceled), ctx.Err() == context.Canceled:
return fmt.Errorf("request canceled: %w", err)
return &mberr.CanceledError{Err: err}
default:
return fmt.Errorf("http request failed: %w", err)
}
Expand Down
14 changes: 12 additions & 2 deletions internal/client/dashboards.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"fmt"
"net/url"

"github.com/andreagrandi/mb-cli/internal/mberr"
)

// ListDashboards retrieves all dashboards.
Expand All @@ -25,7 +27,11 @@ func (c *Client) ListDashboards(ctx context.Context) ([]Dashboard, error) {
func (c *Client) GetDashboard(ctx context.Context, id int) (*Dashboard, error) {
resp, err := c.Get(ctx, fmt.Sprintf("/api/dashboard/%d", id), nil)
if err != nil {
return nil, fmt.Errorf("failed to get dashboard %d: %w", id, err)
return nil, &mberr.RequestError{
Resource: mberr.ResourceDashboard,
Op: fmt.Sprintf("failed to get dashboard %d", id),
Err: err,
}
}

var dashboard Dashboard
Expand All @@ -40,7 +46,11 @@ func (c *Client) GetDashboard(ctx context.Context, id int) (*Dashboard, error) {
func (c *Client) GetDashboardParamValues(ctx context.Context, dashboardID int, paramKey string) (*ParameterValues, error) {
resp, err := c.Get(ctx, fmt.Sprintf("/api/dashboard/%d/params/%s/values", dashboardID, url.PathEscape(paramKey)), nil)
if err != nil {
return nil, fmt.Errorf("failed to get values for dashboard %d parameter %s: %w", dashboardID, paramKey, err)
return nil, &mberr.RequestError{
Resource: mberr.ResourceDashboardParameter,
Op: fmt.Sprintf("failed to get values for dashboard %d parameter %s", dashboardID, paramKey),
Err: err,
}
}

var values ParameterValues
Expand Down
9 changes: 5 additions & 4 deletions internal/config/config.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package config

import (
"fmt"
"os"

"github.com/andreagrandi/mb-cli/internal/mberr"
)

type Config struct {
Expand All @@ -14,18 +15,18 @@ type Config struct {
func LoadConfig() (*Config, error) {
host := os.Getenv("MB_HOST")
if host == "" {
return nil, fmt.Errorf("MB_HOST environment variable is required")
return nil, &mberr.ConfigError{Message: "MB_HOST environment variable is required"}
}

apiKey := os.Getenv("MB_API_KEY")
sessionToken := os.Getenv("MB_SESSION_TOKEN")

if apiKey == "" && sessionToken == "" {
return nil, fmt.Errorf("either MB_API_KEY or MB_SESSION_TOKEN environment variable is required")
return nil, &mberr.ConfigError{Message: "either MB_API_KEY or MB_SESSION_TOKEN environment variable is required"}
}

if apiKey != "" && sessionToken != "" {
return nil, fmt.Errorf("MB_API_KEY and MB_SESSION_TOKEN are mutually exclusive, set only one")
return nil, &mberr.ConfigError{Message: "MB_API_KEY and MB_SESSION_TOKEN are mutually exclusive, set only one"}
}

return &Config{
Expand Down
Loading
Loading