GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [types,connectors] Show library errors through a sanitizing wrapper by bestouff · Pull Request #7857 · feldera/feldera

Conversation

@bestouff

@bestouff bestouff commented Nov 5, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Part of feldera/cloud#2136. Stacked on #7801 (cloud-2136-credentials).

A driver or client error can echo the connection string or the option value it was given, and a log line or a message can interpolate the error anywhere. This adds LibraryError, a wrapper whose Display and Debug are the sanitized text, so that whatever is in scope at a log line or a message is already safe; the raw error stays inside for classification only.

  • feldera_types::library_error::LibraryError: wraps any std::error::Error or anyhow::Error; prints the error and its sources with the credential scrub and the secret registry applied; no source(), so an anyhow chain walker cannot reach the raw text; raw() and downcast_ref() for classification, which also find the original error behind an anyhow chain.
  • The credential scrub moves from the manager to feldera_types::credential_scrub, shared by both sides, and now also covers librdkafka's Invalid value "<v>" for configuration property "<k>", with plain or escaped quotes; a value with no closing part cuts the text off. The manager module re-exports it and keeps the driver-grammar tests.
  • The Postgres input, output and CDC connectors wrap the driver error at each call that returns it: connect, URI parsing, queries, rollback and the backoff classification. The connection monitor is generic over the error type, so the raw type crosses no signature.
  • crates/connectors/clippy.toml disallows tokio_postgres::error::Error; the two classification functions carry an allow with the reason. The rule sees the type only where it is written, so it guards signatures; the wrap at each call is what keeps the raw text out of messages.

Not covered yet: errors from the etl replication library in the CDC connector, and the Kafka, Delta Lake, NATS, DynamoDB and S2 boundaries. They follow in the same shape; the docs say so.

Validation: 22 scrub and wrapper tests, 3 manager tests for the driver grammar, the Postgres connect and classification tests, including an end-to-end test through the real input and CDC connect paths where the driver echoes a password (pasted as an option name) and the error shows the secret reference instead. fmt and clippy clean with warnings denied. Negative validation, each fix broken on purpose: removing the CDC parse wrap or formatting e.raw() in the input connect error fails the end-to-end tests; boxing an anyhow error fails the downcast test; dropping the escaped-quote pass or the cut-off fails the Kafka test; Display printing the raw text fails four tests; a source() that exposes the raw error fails the chain-walker test; dropping one allow makes clippy report the raw type at the signature.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 5, 2026
View reviewed changes
Comment thread crates/connectors/clippy.toml Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 5, 2026
View reviewed changes
Comment thread crates/feldera-types/src/library_error.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mythical-fred-oss

mythical-fred-oss Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

What I ran

Check Result
cargo test -p feldera-types library_error / credential_scrub 6 + 15 passed
cargo test -p pipeline-manager diagnostic_redaction passed
cargo test -p dbsp_connectors --lib the_driver_message_survives passed
cargo clippy -p dbsp_connectors --lib clean
pre-commit run --files <changed> all passed
My QA tests: password only in source() (with unicode and %-encoding), ?password= query param passed
My QA test: downcast_ref after From<anyhow::Error> failed (see inline comment)

Feldera PR gates: tests, docs, licensing, no unsafe, and no new dependencies are all fine. No SQL-type coverage is needed because no data path changed.

Not covered (medium)

  • No test that a real tokio_postgres connect error with a password in the URI comes out redacted end to end through connect() → ControllerError. The only connector test uses a parse error that has no credentials in it.
  • No test for the Kafka scrub on a value with an embedded \", or with no closing quote.
  • The Postgres CDC connector's connect path is still unwrapped (see inline comment on clippy.toml).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bestouff
bestouff added this pull request to stack #7791 November 5, 2026 22:11
@bestouff
bestouff requested a review from mihaibudiu November 5, 2026 22:11
@bestouff
bestouff force-pushed the cloud-2136-library-error branch 2 times, most recently from 5ff1971 to 4ec9378 Compare November 6, 2026 11:01
@bestouff
bestouff marked this pull request as ready for review November 6, 2026 11:09
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 6, 2026
View reviewed changes
Comment thread crates/connectors/src/integrated/postgres/output.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mythical-fred-oss

mythical-fred-oss Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

LGTM, with one test gap noted inline.

What I ran (debug builds)

Check Result
cargo test -p feldera-types (credential_scrub, library_error) 22 passed
cargo test -p dbsp_connectors --lib (the new Postgres tests plus connection_target) 4 passed
cargo test -p pipeline-manager --lib diagnostic_redaction 3 passed
cargo clippy -p dbsp_connectors --lib --tests -D warnings (with the new clippy.toml) clean
Every scrub test the manager used to have still exists after the move yes

Test cases the PR does not cover

  • missing (medium): the output connector has no test for its connection-string parse error. I added one inline; it fails if the wrap is removed.
  • The output connector's rollback-failure path (transaction.rollback()) needs a live database, so it has no unit test. The integration suite should cover it.

Behaviour change: the CDC catalog queries now return LibraryError, whose to_string() includes the server message. Before, it was only db error. So with_setup_hint can now match permission denied from verify_ddl_event_trigger and publication_tables, and add the setup hint. That looks like an improvement. It deserves a line in the commit message, or a test.

Gates: docs updated; no new dependencies, unsafe blocks or CI changes; the commit message is good.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bestouff
bestouff force-pushed the cloud-2136-library-error branch from 4ec9378 to ad67268 Compare November 12, 2026 12:51
@bestouff
bestouff force-pushed the cloud-2136-library-error branch from ad67268 to e230887 Compare November 12, 2026 13:46
@bestouff
bestouff force-pushed the cloud-2136-library-error branch from e230887 to 66e8459 Compare November 12, 2026 14:10
@bestouff
[types,connectors] Show library errors through a sanitizing wrapper
7998fe9
A driver or client error can echo the connection string or the option
value it was given, and a log line or a message can interpolate the
error anywhere. Add `LibraryError`: it keeps the raw error for
classification and prints, as `Display` and `Debug`, the text of the
error and its sources with connection credentials removed and
registered secret values replaced. It has no `source()`, so a chain
walker cannot reach the raw text either. An `anyhow::Error` is kept as
is, so a downcast still finds the original error behind it.

Share the credential scrub between the manager and the pipeline: it
moves to `feldera_types::credential_scrub` and also covers the quoted
value in librdkafka's rejected-option message, with plain or escaped
quotes, and cuts the text off at a value with no closing part.

The Postgres input, output and CDC connectors wrap the driver error at
each call that returns it: connect, URI parsing, queries, rollback and
the backoff classification. A clippy rule in the connectors crate keeps
the raw driver error type out of signatures, with the two
classification functions as the allowed exceptions. The rule only sees
the type where it is written, so a `?` into `anyhow` passes it; the
wrap at each call is what keeps the raw text out of messages.

Test the wrapper's text, its chain, the raw access and the downcast
after an anyhow conversion, the Kafka shapes in the scrub, that the
driver's message survives classification, and, through the real input
and CDC connect paths, that a password the driver echoes shows as its
secret reference. Document the wrapper and the connectors it covers.

Related: feldera/cloud#2136
Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
bestouff force-pushed the cloud-2136-library-error branch from 66e8459 to 7998fe9 Compare November 12, 2026 14:34
@bestouff
[docs] Describe how secrets are kept out of diagnostics
8da31f7
The redaction code has three layers in several crates, and each module
documents only its own layer. A reader had no single place that shows
which layer guards which output.

Add a design page with the data flow from the secret store and the SQL
program to each output, the role and entry points of each layer, and
the known limits. Point to it from the registry module.

Related: feldera/cloud#2136
Signed-off-by: Xavier Bestel <[email protected]>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@mihaibudiu mihaibudiu Awaiting requested review from mihaibudiu

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@bestouff
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.