Skip to content

Add AnalysisError type and wrap all analyzer error paths#4779

Merged
johnelliott merged 6 commits into
mainfrom
enhance/analysis-errors
Apr 15, 2026
Merged

Add AnalysisError type and wrap all analyzer error paths#4779
johnelliott merged 6 commits into
mainfrom
enhance/analysis-errors

Conversation

@johnelliott

@johnelliott johnelliott commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

What this does

Adds a shared error type so analyzer failures carry structured metadata (analyzer type, operation, service). Currently only GCP errors have this; the other 44 analyzers return bare errors.New that get stored as "unknown" in the database.

How to review

Read one file, skip the other 44. There are only 2 things to look at:

1. The new error type (entire abstraction)

pkg/analyzer/analyzers/errors.go -- new file:

// Constants for structured error metadata
const (
    OperationValidateCredentials = "validate_credentials"
    OperationAnalyzePermissions  = "analyze_permissions"
)
const (
    ServiceAPI      = "API"
    ServiceConfig   = "config"
    ServiceDatabase = "Database"
    ServiceOAuth    = "OAuth"
    ServiceCrypto   = "crypto"
)

type AnalysisErrorInfo interface {
    error
    AnalyzerType() string
    Operation() string
    Service() string
    Resource() string
}

type AnalysisError struct {
    analyzerType, operation, service, resource string
    err error
}

func NewAnalysisError(analyzerType, operation, service, resource string, err error) *AnalysisError

2. The pattern every analyzer follows

Every analyzer diff looks like this (Airbrake shown):

// Before:
return nil, err

// After:
return nil, analyzers.NewAnalysisError(
    a.Type().String(),                      // dynamic from analyzer's own type
    analyzers.OperationAnalyzePermissions,  // constant
    analyzers.ServiceAPI,                   // constant
    "",
    err,
)

The original error is wrapped, not replaced (Unwrap() preserves the chain). Analyzer type is derived dynamically via a.Type().String(). Two operation constants are used: OperationValidateCredentials for bad input, OperationAnalyzePermissions for API/DB failures.

All 44 analyzer files are this same 1-3 line change. You can spot-check a few and skip the rest.

Part of a cross-repo change

Order Repo PR What
1 trufflehog this PR Define error type + constants, wrap 44 analyzers
2 integrations #118 Wrap 10 integrations analyzers
3 thog #5724 Scanner consumes the metadata via errors.As()

Test plan

  • go test ./pkg/analyzer/analyzers/... (includes errors_test.go)
  • go build ./pkg/analyzer/analyzers/...

Note

Medium Risk
Touches error-return paths across many analyzers; behavior should be equivalent but changes the concrete error type/message, which could affect downstream error handling or assertions.

Overview
Introduces a new structured AnalysisError (with AnalysisErrorInfo interface) plus shared Operation* and Service* constants so analyzer failures can carry consistent metadata and still Unwrap() to the original error.

Updates many analyzers to wrap both missing credential validation errors and permission/API/DB analysis failures with analyzers.NewAnalysisError(...) instead of returning raw errors, standardizing how failures are reported and stored.

Reviewed by Cursor Bugbot for commit 9dbe823. Bugbot is set up for automated code reviews on this repo. Configure here.

@johnelliott johnelliott requested review from a team February 27, 2026 20:54
@CLAassistant

CLAassistant commented Feb 27, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Comment thread pkg/analyzer/analyzers/airbrake/airbrake.go
Comment thread pkg/analyzer/analyzers/airbrake/airbrake.go Outdated
Comment thread pkg/analyzer/analyzers/airtable/airtablepat/airtable.go Outdated
@johnelliott johnelliott force-pushed the enhance/analysis-errors branch from 6ef6651 to 409135f Compare March 4, 2026 23:39
Comment thread pkg/analyzer/analyzers/errors.go
@johnelliott johnelliott force-pushed the enhance/analysis-errors branch from 793a1b5 to b747fd9 Compare March 6, 2026 22:40
Comment thread pkg/analyzer/analyzers/errors.go
@johnelliott johnelliott force-pushed the enhance/analysis-errors branch from b747fd9 to 0f66014 Compare March 11, 2026 17:02
@johnelliott johnelliott force-pushed the enhance/analysis-errors branch from 0f66014 to 9fe25fe Compare April 9, 2026 16:43

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9fe25fe. Configure here.

Comment thread pkg/analyzer/analyzers/github/github.go
Introduce a shared error type that provides structured metadata
(analyzer type, operation, service, resource) for analysis failures.
This allows the scanner to extract context from errors without
depending on concrete types.
Batch A: Airbrake, Anthropic, Asana, DigitalOcean, DockerHub,
ElevenLabs, Fastly, Groq, HuggingFace, Mailchimp, Mailgun, Mux,
Netlify, Ngrok, Notion, OpenAI, Opsgenie, Posthog, Postman,
Sendgrid, Sourcegraph.

Wraps credential validation errors with operation
"validate_credentials" and AnalyzePermissions errors with
operation "analyze_permissions".
Batch B (OAuth/multi-credential): airtableoauth, airtablepat, datadog,
dropbox, figma, launchdarkly, plaid
Batch C (Complex): bitbucket, databricks, github, gitlab, jira, monday,
planetscale, shopify, slack, square, stripe, twilio
Batch D (Database): mysql, postgres (service: Database)
Batch E (PrivateKey): privatekey (service: crypto)
Address PR feedback: replace hardcoded analyzer type strings with
a.Type().String() and replace raw operation/service strings with
package-level constants (OperationValidateCredentials,
OperationAnalyzePermissions, ServiceAPI, ServiceConfig, etc.).
Conditionally include "(resource: ...)" only when non-empty,
avoiding cluttered messages like "... (resource: ): ..." that
appear for the majority of analyzers that don't set a resource.
@johnelliott johnelliott force-pushed the enhance/analysis-errors branch from e21487f to 9dbe823 Compare April 14, 2026 22:53
@johnelliott johnelliott dismissed shahzadhaider1’s stale review April 14, 2026 23:50

Changes were addressed and reviewer subsequently approved. Dismissing stale change request.

@hxnyk hxnyk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤠

@johnelliott johnelliott merged commit 761be88 into main Apr 15, 2026
14 checks passed
@johnelliott johnelliott deleted the enhance/analysis-errors branch April 15, 2026 00:18
MuneebUllahKhan222 pushed a commit that referenced this pull request May 29, 2026
* Add AnalysisError type and AnalysisErrorInfo interface

Introduce a shared error type that provides structured metadata
(analyzer type, operation, service, resource) for analysis failures.
This allows the scanner to extract context from errors without
depending on concrete types.

* Wrap errors in simple API analyzers with AnalysisError

Batch A: Airbrake, Anthropic, Asana, DigitalOcean, DockerHub,
ElevenLabs, Fastly, Groq, HuggingFace, Mailchimp, Mailgun, Mux,
Netlify, Ngrok, Notion, OpenAI, Opsgenie, Posthog, Postman,
Sendgrid, Sourcegraph.

Wraps credential validation errors with operation
"validate_credentials" and AnalyzePermissions errors with
operation "analyze_permissions".

* Wrap errors in remaining analyzers with AnalysisError (Batches B-E)

Batch B (OAuth/multi-credential): airtableoauth, airtablepat, datadog,
dropbox, figma, launchdarkly, plaid
Batch C (Complex): bitbucket, databricks, github, gitlab, jira, monday,
planetscale, shopify, slack, square, stripe, twilio
Batch D (Database): mysql, postgres (service: Database)
Batch E (PrivateKey): privatekey (service: crypto)

* Use Type().String() and constants for NewAnalysisError calls

Address PR feedback: replace hardcoded analyzer type strings with
a.Type().String() and replace raw operation/service strings with
package-level constants (OperationValidateCredentials,
OperationAnalyzePermissions, ServiceAPI, ServiceConfig, etc.).

* Omit empty resource parenthetical from AnalysisError messages

Conditionally include "(resource: ...)" only when non-empty,
avoiding cluttered messages like "... (resource: ): ..." that
appear for the majority of analyzers that don't set a resource.

* Wrap no-data error path in GitHub analyzer with AnalysisError
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants