fix(table): preserve error codes from db layer in Find methods

[dev-arya23]

Summary

Table.Find, FindMany, FindManyWithOpts and the equivalent CachedTable.DBFind, DBFindMany, DBFindManyWithOpts unconditionally wrapped every error from the db layer as NotFound, destroying the error code that interpretMongoError had already set correctly.

This caused callers using coreerrors.IsNotFound(err) to treat transient storage failures (connection resets, timeouts, auth failures) as genuine "document does not exist" results.

Root cause

// Before: wraps ALL errors as NotFound
err := t.col.FindOne(ctx, key, &data)
if err != nil {
    return nil, errors.Wrapf(errors.NotFound, "failed to find entry with key %v: %s", key, err)
}

The db layer's interpretMongoError already correctly maps mongo.ErrNoDocuments → NotFound and passes other errors through with no error code. The Table layer then re-wrapped everything as NotFound, making transient failures indistinguishable from true misses.

Fix

// After: preserve recognized error codes, wrap unrecognized as Internal
err := t.col.FindOne(ctx, key, &data)
if err != nil {
    if errors.GetErrCode(err) != errors.Unknown {
        return nil, err // already a typed error from the db layer
    }
    return nil, errors.Wrapf(errors.Internal, "failed to find entry with key %v: %s", key, err)
}

If the error from the db layer already carries a recognized error code (e.g., NotFound from interpretMongoError for ErrNoDocuments), pass it through unchanged. Otherwise wrap as Internal so callers can distinguish transient failures from true misses.

Changes

• errors/const.go: Add Internal error code (6)
• errors/errors.go: Add IsInternal helper
• table/generic.go: Fix Find, FindMany, FindManyWithOpts
• table/cached_generic.go: Fix DBFind, DBFindMany, DBFindManyWithOpts

Affected methods (6 total)

Type	Method	Before	After
Table	Find	All errors → NotFound	Typed errors pass through, raw errors → Internal
Table	FindMany	All errors → NotFound	Same
Table	FindManyWithOpts	All errors → NotFound	Same
CachedTable	DBFind	All errors → NotFound	Same
CachedTable	DBFindMany	All errors → NotFound	Same
CachedTable	DBFindManyWithOpts	All errors → NotFound	Same

Impact on consumers

Once this fix lands, existing consumers' transient-failure branches become reachable without any code changes on their side:

• Callback reconciler (oauth-connection-manager): Reconcile path at coreerrors.IsNotFound(err) will correctly retry on transient DB failures instead of silently dropping the key
• Provider reconciler: IsNotFound check now only triggers on genuine misses (behavior unchanged since re-upsert is idempotent)
• Cold-start scan: FindMany errors are already fatal to boot — behavior unchanged
• CachedTable.callback: errors.IsNotFound(err) check for cache eviction now correctly distinguishes deletes from transient failures

Backward compatibility

• IsNotFound(err) still returns true for genuine missing documents — no consumer behavior change on the happy path
• New Internal error code and IsInternal helper are additive — no breaking changes
• Existing integration tests should pass unchanged since they test against real MongoDB where FindOne for missing docs returns ErrNoDocuments → NotFound (same as before)

Fixes agentic-core/core#48

Summary by CodeRabbit

• New Features

  • Cached tables now support configurable filters and watch pipelines for fine-grained control over initial data loading and change stream event filtering.

• Improvements

  • Database query operations now provide more precise error classification, distinguishing between transient storage failures and missing data rather than treating all failures uniformly.

[coderabbitai]

Warning

Review limit reached

@dev-arya23, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 51 minutes and 42 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2d6a5999-fb4a-471b-b3ab-ceecd16627a4

📥 Commits

Reviewing files that changed from the base of the PR and between 805060b and 2708eb8.

📒 Files selected for processing (2)

• errors/errors_test.go
• table/cached_generic.go

Walkthrough

Adds an Internal (ErrCode = 6) constant and IsInternal predicate to the errors package. Extends CachedTableConfig with Filter and WatchPipeline fields and corresponding WithFilter/WithWatchPipeline options. Wires those fields through InitializeWithConfig (preflight validation, watch registration, eager load) and ReconcilerGetAllKeys. Updates Find/FindMany error handling in both generic.go and cached_generic.go to preserve recognized DB-layer error codes and wrap errors.Unknown as errors.Internal instead of errors.NotFound.

Changes

Internal error code, CachedTable filter scoping, and DB error preservation

Layer / File(s)	Summary
Internal ErrCode constant and IsInternal predicate errors/const.go, errors/errors.go	Adds Internal ErrCode = 6 to the constant block and exports IsInternal(err error) bool backed by GetErrCode.
CachedTableConfig filter/pipeline fields and options table/cached_generic.go	Extends CachedTableConfig with Filter any and WatchPipeline any fields, stores them on CachedTable, and introduces WithFilter and WithWatchPipeline functional options with documentation.
Filter wiring in InitializeWithConfig and ReconcilerGetAllKeys table/cached_generic.go	Stores filter/pipeline on the instance, adds col.Count preflight validation, switches col.Watch to use t.watchPipeline, passes t.filter to eager key loading (returning error instead of panicking on failure), and scopes ReconcilerGetAllKeys enumeration to t.filter.
DB error code preservation in Find and FindMany paths table/generic.go, table/cached_generic.go	Updates Table.Find, Table.FindManyWithOpts, DBFind, DBFindMany, and DBFindManyWithOpts to return recognized DB-layer error codes as-is and wrap errors.Unknown as errors.Internal instead of always wrapping as errors.NotFound.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 Hoppin' through the error maze,
Internal codes now get their praise!
With filters set and pipelines scoped,
No more NotFound blindly hoped.
The cache knows what and where to seek —
This rabbit's code review? Magnifique! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately and concisely describes the primary change: preserving error codes from the database layer in Find methods, which is the core objective.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

errors/errors.go (1)

90-94: ⚡ Quick win

Add test coverage for the new IsInternal predicate.

IsInternal is exported and part of the same predicate contract as IsNotFound/IsAlreadyExists; adding a small assertion in errors/errors_test.go would lock this behavior and prevent regressions.

Suggested test addition

 func Test_ErrorValidations(t *testing.T) {
@@
 	err = Wrapf(NotFound, "%s", "test wrapf error from errors pkg")
 	if !IsNotFound(err) {
 		t.Errorf("expected error type Not Found")
 	}
+
+	err = Wrap(Internal, "internal failure")
+	if !IsInternal(err) {
+		t.Errorf("expected error type Internal")
+	}
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@errors/errors.go` around lines 90 - 94, The IsInternal function is exported
and part of the error predicate contract, but it currently lacks test coverage.
Add test cases in errors_test.go that verify IsInternal returns true when an
error has the Internal error code and false otherwise, following the same
testing pattern used for existing predicates like IsNotFound and
IsAlreadyExists.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@table/cached_generic.go`:
- Around line 224-227: The error handling for the Count call in the filter
validation block and the eager-load block are currently wrapping all failures
with fixed error codes (InvalidArgument and Unknown respectively), which masks
the actual error types for transient failures like storage, auth, or network
errors. Instead of always wrapping with a fixed error code, check if the error
returned from col.Count is already a properly classified error and preserve its
error code. Only apply the InvalidArgument wrap if the error genuinely indicates
a filter validation problem. Apply the same approach to the eager-load error
handling to preserve the original error codes rather than always wrapping as
Unknown. This ensures that transient infrastructure failures are properly
distinguishable from actual validation errors.

---

Nitpick comments:
In `@errors/errors.go`:
- Around line 90-94: The IsInternal function is exported and part of the error
predicate contract, but it currently lacks test coverage. Add test cases in
errors_test.go that verify IsInternal returns true when an error has the
Internal error code and false otherwise, following the same testing pattern used
for existing predicates like IsNotFound and IsAlreadyExists.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6b8622eb-ec5b-418e-9692-7e8e175a463e

📥 Commits

Reviewing files that changed from the base of the PR and between 122c276 and 805060b.

📒 Files selected for processing (4)

• errors/const.go
• errors/errors.go
• table/cached_generic.go
• table/generic.go