Skip to content

feat: Expose error classifier#2539

Open
bbernays wants to merge 2 commits into
mainfrom
expose-error-classifier
Open

feat: Expose error classifier#2539
bbernays wants to merge 2 commits into
mainfrom
expose-error-classifier

Conversation

@bbernays

@bbernays bbernays commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

This PR exposes a way for plugins to capture and classify errors at a plugin level rather than having to handle it on a per resolver basis.

For example this can be utilized in our AWS plugin to handle 404s or AWS specific errors without having to replace or wrap the error handling in every single table resolver

Example PR that uses this functionality: https://github.com/cloudquery/cloudquery-private/pull/13197

@bbernays bbernays requested a review from a team as a code owner July 15, 2026 16:01
@bbernays bbernays requested a review from disq July 15, 2026 16:01
Comment thread scheduler/queue/worker.go Outdated
Comment thread scheduler/resolvers/resolvers.go Outdated

@murarustefaan murarustefaan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is good overall, but requesting a review from @erezrokah as well before merging.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a plugin-configurable error classification hook so resolver errors can be suppressed at the scheduler/plugin level (e.g., ignoring expected 404s) rather than handled in each resolver.

Changes:

  • Added schema.ErrorClassifier (+ supporting ErrorEvent/ErrorPhase) and threaded it through scheduler/resolver execution paths.
  • Introduced scheduler.WithErrorClassifier(...) and wired the classifier into DFS + shuffle-queue worker execution and resource-chunk resolution.
  • Added tests verifying default behavior (raised) vs suppressed behavior across strategies and resolver phases.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
schema/error_classifier.go Adds the new public error-classifier API types and nil-safe helper.
scheduler/scheduler.go Re-exports schema classifier types/phases and adds WithErrorClassifier.
scheduler/scheduler_shuffle_queue.go Passes classifier into the shuffle-queue scheduler.
scheduler/scheduler_dfs.go Consults classifier for table-resolver errors and passes it into chunk resolution.
scheduler/resolvers/resolvers.go Adds classifier plumbing for chunk/column/pre/post resolver error handling.
scheduler/queue/scheduler.go Adds option + field to carry classifier into queue workers.
scheduler/queue/worker.go Consults classifier for table-resolver errors and passes it into chunk resolution.
scheduler/scheduler_error_classifier_test.go Verifies scheduler-level suppression vs default SyncError emission across strategies.
scheduler/resolvers/resolvers_test.go Updates existing resolver tests for new function signature.
scheduler/resolvers/error_classifier_test.go Adds phase coverage tests ensuring suppression affects metrics and event metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +42 to +45
// ErrorClassifier reports whether a resolver error should be suppressed rather than
// raised. Suppressed errors are logged at debug level and are not counted in error
// metrics or emitted as a SyncError message. A nil ErrorClassifier raises every error.
// It is not consulted for primary key calculation or validation errors.
}

func ResolveResourcesChunk(ctx context.Context, logger zerolog.Logger, m *metrics.Metrics, table *schema.Table, client schema.ClientMeta, parent *schema.Resource, chunk []any, c *caser.Caser) []*schema.Resource {
func ResolveResourcesChunk(ctx context.Context, logger zerolog.Logger, m *metrics.Metrics, table *schema.Table, client schema.ClientMeta, parent *schema.Resource, chunk []any, c *caser.Caser, classifier schema.ErrorClassifier) []*schema.Resource {
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.

3 participants