NCBC-4296: Honour the configured redaction level in error contexts by davidkelly · Pull Request #181 · couchbase/couchbase-net-client

davidkelly · 2026-09-04T21:54:25Z

ClusterOptions.RedactionLevel redacted the log stream but not exception error contexts, which serialized every field raw — so a customer who turned redaction on for compliance got logs that looked clean while document keys, statements and query parameters went out untouched. This applies the redactor at every error-context construction site, matching Java's classification, and reuses the user/meta/system classification the SDK's log paths already use. Redaction is off by default so nothing changes unless a customer opted in; for those who did, ex.Context.DocumentKey now returns <ud>the-key</ud>, which needs a release note.

What's classified as what

Classification	Fields
user data (`<ud>`, redacted at Partial)	document keys, statements, query parameters, search queries
metadata (`<md>`, redacted at Full)	bucket, scope, collection, index, design-doc and view names
system data (`<sd>`, redacted at Full)	endpoints, management URIs

Not redacted, deliberately: ClientContextId, which support correlates against server logs, and the server-authored Message / Errors text.

Judgement calls worth a reviewer's attention

Redaction happens at assignment, not at render. This is the one real design choice, and it is the fail-closed one. The stored value is tagged, so every consumer is covered — ToString(), a customer's own field logging, a debugger watch window — including consumers that do not exist yet. Render-time redaction is fail-open by comparison: every render path has to be redaction-aware, and a missed one leaks silently. This PR found two such paths already. ToString() needed the relaxed encoder to emit literal tags at all, and the now-deleted ToJsonString() had the same bug unnoticed for a year — under render-time redaction that would have been a live leak rather than a cosmetic one. NCBC-4297(c) will add a third when it appends Context to Message.

The cost is that the raw value is not recoverable from the context. Nothing in the SDK reads these fields back — the only internal readers are Status and Errors/Code — and for someone who enabled redaction for compliance, an unrecoverable key is closer to the intent than to a loss. If that ever needs to change, an internal raw field alongside the public tagged property is additive.
There is no spec for any of this, and the other SDKs do not settle it. Redaction appears once in all 45 SDK RFCs, as an aside about a tracing tag in 0035; the <ud>/<md>/<sd> convention comes from the server side. Go checks the redaction level at the call site but never redacts an error context at all. Java wraps at assignment and renders in RedactableArgument.toString(), which is instructive rather than precedent: a 2019 "exact syntax yet to be determined" in that one method leaves redactMeta and redactSystem emitting untagged values to this day, at every call site, while every call site still looks correct. Centralised rendering makes one wrong decision invisible everywhere.
Java classifies identically but only ever emits <ud>. ReducedKeyValueErrorContext uses redactUser for the document id and redactMeta for bucket, scope and collection, exactly as the table above does, but its render step returns meta and system values raw. .NET has emitted <md> and <sd> in log lines since NCBC-3079 in 2021, so the table follows .NET's own established convention, not Java's behaviour. Worth knowing before comparing the two.
Null and empty pass through untouched. Redacted<T>.ToString() renders null as "", which would turn an absent field into a present, empty one, and tagging an empty value yields a useless <ud></ud>. GetClusterMap passes string.Empty as the bucket name, so this is reachable.
SelectBucket is the one operation whose Key is not a document key — it holds the bucket name, so it is metadata; as user data it would be stripped at Partial, losing a diagnostic Couchbase treats as safe there. The rule lives in one helper (RedactorExtensions.OperationKey) that all nine sites route through: the three error contexts (ClusterNode, ResponseStatusExtensions, RetryOrchestrator), the timeout message in ThrowHelper, and the five ClusterNode log lines SelectBucket reaches. Previously the log line and the message classified that same value as user data while the context called it metadata.
Error contexts serialize with the relaxed JSON encoder. System.Text.Json escapes < and > by default, which turned every tag into an escape sequence — semantically correct and operationally useless, since cblogredaction matches the tags textually. Side effect: non-ASCII in these contexts is no longer escaped either.
EventingFunctionErrorContext needs nothing. It is never constructed anywhere in the SDK, and its only non-Message field is [JsonIgnore(Always)].

Behaviour change

For anyone who has enabled redaction, error-context fields hold the tagged value. The affected properties are all [InterfaceStability(Level.Uncommitted)], and nothing inside the SDK reads them back — the only internal consumers read Status (ClusterContext, CouchbaseBucket) and Errors/Code (transactions' ConvertQueryError), neither of which is redacted.

Release note required. It should say the fix covers the classic path, for the reason below.

Known gap: the couchbase2:// path is not covered

GenericErrorContext on the Stellar path has the same defect and is untouched here. StellarRetryHandler copies the gRPC ResourceInfo detail into Fields["ResourceName"] raw, and when the sibling ResourceType is "document" that value is the document key — so DocumentNotFoundException and DocumentExistsException over couchbase2:// still write keys unredacted into the log. This is a real gap, not a scoping technicality — tracked as NCBC-4300, targeting 3.10.0.

It is left out because it is a different fix rather than more of the same:

No redactor exists on that path. StellarRetryHandler holds no redactor, and the IRedactor on StellarCluster has never been invoked by Stellar code. It needs DI and constructor changes, not a classification change.
Different serialization problem. GenericErrorContext.ToString() serializes an untyped Dictionary<string, object> reflectively, with no serializer context and no options — so it needs the relaxed-encoder fix separately, and raises a trimming/AOT question the four source-generated contexts did not.
ResourceName's classification depends on the sibling ResourceType value, the same value-provenance shape as SelectBucket's Key, so the helper added here is the right model for it.
The classification test below cannot reach an untyped bag; covering it needs per-key classification.

ResourceType "path" (a subdoc path) also has no classic equivalent in any context, so whether it is user data or metadata is an open question for that ticket, and probably a cross-SDK one.

Testing

30 new tests. ErrorContextRedactionTests covers None/Partial/Full and tag survival through ToString(). ErrorContextRedactionClientTests drives the query, search, analytics, view and management paths end-to-end through mocked HTTP failures, so every context type has a site pinned. OperationKeyRedactionTests pins the SelectBucket rule in both the context and the timeout message, plus the counterparts proving an ordinary Get is still user data.

ErrorContextClassificationTests is the one worth understanding. Redaction applied field by field has exactly one failure mode — a field nobody classified — and value-based tests cannot catch it, because a new unredacted field simply is not asserted on. So every string field on all six context types is declared as either redacted or deliberately raw, and checked two ways: a reflection test that fails until a new field is listed, and a behavioural test that drives each real construction path at Full and asserts redacted fields come back tagged and raw ones do not. 22 of the 31 string fields are populated by a real path; the rest are covered by the reflection half. A third test scans the assembly for IErrorContext implementers, so a whole new context type cannot arrive unlisted either - the two deliberately left alone, EventingFunctionErrorContext and Stellar's GenericErrorContext, are named there with the reason each needs nothing.

CI: 3021 pass on net8.0 and net10.0, zero failures. All five SDK target frameworks build with no warnings.

Reading the commits

The first three are the fix. The last six respond to review: the operation-key helper, the classification tests, three cleanups — a real redactor in tests that build contexts, one home for the redaction-safe serializer settings, and one copy of the HTTP fixture helpers — and a last pass that deletes CouchbaseException.ToJsonString().

That method serialized a context with the default HTML-escaping encoder, so its tags would have come out as escape sequences. It turned out to have no caller: it is internal, InternalsVisibleTo names only the two test assemblies, and the FIT performer its comment pointed at is a separate assembly. It goes, along with InterfaceRuntimeTypeConverter, its only consumer.

Follow-up recorded on NCBC-4297

QueryErrorContext and AnalyticsErrorContext have no ToString() override, because both hold List<Error> and Error carries a [JsonExtensionData] IDictionary<string, object> that can contain Newtonsoft JTokens — not serializable through the source-generated context, which is why their DebuggerDisplay routes through SerializeWithFallback under [RequiresUnreferencedCode]. That attribute cannot go on a ToString() override.

This must be fixed before NCBC-4297(c) appends Context to Message, or every query and analytics exception message would end in a bare type name. The ticket's (c) currently assumes all contexts already render as compact JSON.

🤖 Generated with Claude Code

Motivation ========== Setting ClusterOptions.RedactionLevel redacted user data in the log stream but not in exception error contexts, which serialized every field raw. Exception dumps commonly land in the same logs, so redaction a customer enabled for compliance was silently defeated on that path - worse than no redaction, because the logs look clean. None of the error-context construction sites applied the redactor. Java redacts at every put (redactUser/redactMeta/redactSystem); we did not. Modification ============ Redact at assignment across all 33 construction sites, matching Java, using the existing user/meta/system split already applied in the log paths: document keys, statements, parameters and search queries as user data; bucket, scope, collection, index and view names as metadata; endpoints as system data. TypedRedactor gains UserDataString/MetaDataString/SystemDataString for the contexts, which store plain strings rather than Redacted<T>, mirrored as IRedactor extension methods so every call site reads the same regardless of which abstraction the class holds. They pass null and empty through untouched, since Redacted<T>.ToString() renders null as "" and tagging an empty value yields a useless "<ud></ud>". QueryClient, SearchClient and AnalyticsClient now take an IRedactor, matching ViewClient. ResponseStatusExtensions.CreateException and RetryOrchestrator.CreateKeyValueErrorContext take one as a parameter. SelectBucket carries the bucket name in Key rather than a document key, so that one is tagged as metadata - as user data it would be stripped at Partial redaction. Not redacted, deliberately: ClientContextId, which support correlates against server logs, and the server-authored Message and Errors text. Results ======= Error contexts now tag user data whenever redaction is enabled. No change with the default RedactionLevel.None, where the helpers return the same string instance. 3000 unit tests pass. Nine new tests cover None/Partial/Full, tag survival through ToString(), null and empty preservation, and the query and search client paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-04T21:55:35Z

FIT performer image

Published:

ghcr.io/couchbase/dotnet-fit-performer:NCBC-4296

Run FIT locally against this PR:

fit run preset <preset-name> --performer dotnet-fit-performer:NCBC-4296

Or run it from the workflow here.

Note

Each push to this PR replaces the image and edits this comment. The image is deleted 7 days after the last push.

Copilot

🟡 Changes recommended

The serialization disclosure and incorrect SelectBucket classification must be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Applies configured redaction levels to exception error contexts across KV, query, analytics, search, views, and management operations.

Changes:

Adds string redaction helpers that preserve null and empty values.
Injects redactors into service clients and context construction paths.
Adds redaction-focused tests and shared test utilities.

Review findings:

Critical (2 votes): Serialized exception text escapes redaction tags, preventing tooling from recognizing them and leaving sensitive values exposed. Production serialization and raw-output tests must be corrected.
Moderate (2 votes): ClusterNode classifies the SelectBucket bucket name as user data instead of metadata on its timeout path.
Nit (1 vote): Add representative redaction tests for Analytics, View, and Management error contexts.

File summaries

File	Description
`tests/Couchbase.UnitTests/Utils/TestRedactor.cs`	Adds reusable test redactors.
`tests/Couchbase.UnitTests/Utils/MockedHttpClients.cs`	Supports redactor injection in mocked clients.
`tests/Couchbase.UnitTests/Search/SearchClientTests.cs`	Updates search client construction.
`tests/Couchbase.UnitTests/Query/QueryClientTests.cs`	Updates query client construction.
`tests/Couchbase.UnitTests/Management/SearchClientTests.cs`	Updates management search tests.
`tests/Couchbase.UnitTests/Management/Query/QueryIndexManagerTests.cs`	Updates query manager tests.
`tests/Couchbase.UnitTests/Core/Exceptions/ErrorContextRedactionTests.cs`	Tests context redaction behavior.
`tests/Couchbase.UnitTests/Analytics/AnalyticsClientTests.cs`	Updates analytics client construction.
`src/Couchbase/Views/ViewClient.cs`	Redacts view metadata.
`src/Couchbase/Search/SearchClient.cs`	Redacts search context data.
`src/Couchbase/Query/QueryClient.cs`	Redacts query statements and parameters.
`src/Couchbase/Management/Search/SearchIndexManager.cs`	Redacts management URIs.
`src/Couchbase/Management/Collections/CollectionManager.cs`	Redacts collection-management URIs.
`src/Couchbase/Management/Buckets/BucketManager.cs`	Redacts bucket-management URIs.
`src/Couchbase/Core/Retry/RetryOrchestrator.cs`	Redacts retry-generated KV contexts.
`src/Couchbase/Core/Logging/TypedRedactor.cs`	Adds string redaction helpers.
`src/Couchbase/Core/Logging/RedactorExtensions.cs`	Adds equivalent `IRedactor` extensions.
`src/Couchbase/Core/IO/ResponseStatusExtensions.cs`	Redacts status-generated KV contexts.
`src/Couchbase/Core/ClusterNode.cs`	Redacts node-generated KV contexts.
`src/Couchbase/Analytics/AnalyticsClient.cs`	Redacts analytics statements and parameters.

Review details

Suppressed comments (1)

tests/Couchbase.UnitTests/Core/Exceptions/ErrorContextRedactionTests.cs:122

The tests currently cover only KV, Query, and Search contexts, despite this claim that every remaining context type is pinned. The changed Analytics, View, and Management error-context paths have no redaction assertions, so regressions or incorrect classifications there would remain green. Add representative failure-path tests for those context types.

    /// The KV path is covered above by exercising CreateException directly. These drive the HTTP
    /// service clients so that each remaining context type has at least one site pinned - without
    /// them, a future edit to any of the ~30 assignment sites drops redaction with green tests.

Files reviewed: 20/20 changed files
Comments generated: 2
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

timeout path Motivation ========== Two findings from review of the previous commit. System.Text.Json escapes '<' and '>' by default, to keep JSON safe to embed in HTML. That turned every redaction tag into an escape sequence, so cblogredaction - which finds the tags textually - could not match them, and a redaction pass that appeared to run left the value in place. The tags were semantically correct and operationally useless. Separately, SelectBucket reaches the KV timeout context in ExecuteOp as well as ResponseStatusExtensions, and only the latter had the opcode-based classification. Its Key is the bucket name, so on that path it was tagged as user data and would be stripped at Partial redaction. Modification ============ Each serializer context that carries an error context gains RedactionSafeOptions: its own settings plus JavaScriptEncoder.UnsafeRelaxedJsonEscaping, which escapes only what JSON requires. The source-generated resolver is carried over so serialization stays trim- and AOT-safe, and the four contexts with a ToString() bind a cached JsonTypeInfo from it. The options are created lazily rather than in a field initializer, because Default is not yet constructed while the serializer context runs its own static initialization. Apply the SelectBucket opcode check in the ClusterNode timeout context too, matching ResponseStatusExtensions. Results ======= Redacted contexts now serialize as <ud>key</ud> rather than an escape sequence, so log-redaction tooling can act on them. Note this also stops non-ASCII being escaped in these contexts, which was happening regardless of redaction. 3002 unit tests pass. The ToString test now asserts the raw string rather than a parsed value, since the escaping is precisely what would break the tooling while still round-tripping through a parser. Added analytics and view coverage, and corrected a comment that claimed more coverage than existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Motivation ========== Review noted the tests claimed to pin every remaining context type but covered only KV, query and search. Analytics and view were added in the previous commit; management, which has the most assignment sites at fourteen, was still uncovered. Modification ============ Drive CollectionManager against a failing management endpoint with a real redactor and assert the management URI is tagged as system data. Note the existing CollectionManagerTests scaffold passes Mock<IRedactor>, whose methods return null - a context built through it silently gets a null Statement rather than a redacted one. This test uses TestRedactor.Full instead, which is why it can assert on the value at all. Results ======= Every error-context type now has at least one site pinned: key value, query, search, analytics, view and management. 3003 unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

Cross-cutting redaction and serialization changes across many paths warrant final human review.

Review details

Files reviewed: 26/26 changed files
Comments generated: 0 new
Review effort level: Balanced

Motivation ========== The SelectBucket carve-out - its Key holds the bucket name rather than a document key, so it is metadata - was duplicated across the two error-context sites, and contradicted by the log line and the exception message on the same code path, which both tagged that same value as user data. So at Partial redaction the bucket name was stripped from the timeout message and from the log line but kept in the context. The carve-out reached only one of the three places the value appears, and the rule living in two files is what caused it to be missed once already. Modification ============ Add RedactorExtensions.OperationKey/OperationKeyString as the single place that decides how an operation's key is classified, and route all six sites through it: the two error contexts, ThrowHelper's timeout message, and the four ClusterNode log lines that SelectBucket reaches through ExecuteOp. RetryOrchestrator's three sites are left alone, since SelectBucket never goes through the retry orchestrator. Kept as an extension on TypedRedactor so that Core.Logging does not take a dependency on Core.IO.Operations. Unlike the *String helpers it deliberately does not special-case an empty key: internal operations leave Key at string.Empty and have always logged an empty tag. Results ======= The same value can no longer be classified two ways, and a new call site cannot silently drop the rule. Five new tests pin both shapes and both directions - SelectBucket is metadata in the error context and in the timeout message, an ordinary Get is still user data in both. Verified they fail when the rule is removed from the helper. 3008 unit tests pass on net8.0 and net10.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Motivation ========== Redaction is applied field by field at around thirty construction sites, so the failure mode of the design is a field that nobody classified - which is what happened twice during review of this branch. Value-based tests cannot catch it: a new unredacted field simply is not asserted on, and the suite stays green. Modification ============ Declare the classification of every string field on all six error context types in one table, then check it two ways. A reflection test fails until a new field is listed as either redacted or deliberately raw, and its message says how to decide. A behavioural test drives each real construction path at Full and asserts that redacted fields come back tagged and raw ones do not - so it also catches over-redaction of ClientContextId, which support correlates against server logs. The KV driver uses a mocked IOperation so that DispatchedFrom and DispatchedTo are populated: they are read-only on OperationBase and only written during real dispatch, so until now they were asserted only as null. Twenty-two of the thirty-one string fields come back populated; the rest are covered by the reflection test alone, which is noted in the table. Share the response and fixture helpers with the existing client tests rather than copying them. Results ======= Adding a field to an error context now forces a redaction decision, and leaving one unredacted at a construction site fails. Verified both halves fail when broken: dropping DispatchedFrom from the table, and unredacting it in ResponseStatusExtensions. 3020 unit tests pass on net8.0 and net10.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Motivation ========== Mock<IRedactor> returns null from every method, so a component built with one now silently produces a null error-context field rather than a redacted value. Nothing asserts on those fields today, which is precisely the failure mode: the trap is invisible until someone writes the test that should have caught a regression. Modification ============ Switch the two sites whose redactor output the SDK actually stores - CollectionManager, which puts the management URI in the context, and ViewClient, which puts the design-document and view names there - to TestRedactor.None, a real redactor with redaction disabled. The remaining Mock<IRedactor> sites are left alone deliberately: they pass a redactor to components that only hand it to ILogger, so a null return changes nothing. No test verifies calls on a redactor mock. Results ======= A test that builds one of these two components and asserts on a context field now sees the value rather than null. 3020 unit tests pass on net8.0 and net10.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Motivation ========== The lazily created relaxed-encoder options were duplicated verbatim across the two serializer contexts, and the JsonTypeInfo binding plus its three-line explanatory comment across the four error contexts that override ToString(). Six copies of one idea, and the reason for it was stated twice and abbreviated four times. Modification ============ Add RedactionSafeJson, which owns both the settings and the reasoning: Create() applies the relaxed encoder to a context's options, TypeInfo() binds a type from them. Each serializer context keeps its own lazily created options, now three lines using LazyInitializer, and each error context binds its cached type info in one line. The null-forgiving operator on those two properties is needed because the netstandard targets lack the NotNull annotation that the modern ones have on EnsureInitialized - caught by building every TFM, since only the netstandard legs fail on it. No behaviour change: same options, same deferred creation, same cached JsonTypeInfo per context. Results ======= 137 lines become 44. The redaction tag test still asserts the literal markers survive ToString(), so the encoder is still applied. 3020 unit tests pass on net8.0 and net10.0; all five TFMs of the SDK build with no warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Motivation ========== Reading an embedded fixture into a buffer and building a queue of canned responses was written three times: twice in the new redaction tests and once in RetryOrchestratorTests, where the fixture read was itself repeated four times behind a NET8_0_OR_GREATER conditional. Modification ============ Move both helpers to HttpFixtures in the test Utils folder and pull them in with a static using, so existing call sites read unchanged. The shared read loop works on every target, which removes the four conditional blocks; reading an embedded resource synchronously is not worth a per-framework branch. Test_Views loses the using block that scoped the stream, so its body is dedented a level. Results ======= Three copies become one, and RetryOrchestratorTests drops 60 lines. 3020 unit tests pass on net8.0 and net10.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jeffrymorris · 2026-09-08T23:17:48Z

            var config = await ExecuteInternalOperationAsync(ConnectionPool, configOp,
                ExecuteOpImmediatelyAsync,
-                static (status, op) =>
+                (status, op) =>


The missing static to capture redactor will make the previously cached delegates into new closure allocations per call. It's a performance regression.

jeffrymorris · 2026-09-08T23:27:55Z


-        public static Exception CreateException(this ResponseStatus status, IOperation op, string bucketName)
+        public static Exception CreateException(this ResponseStatus status, IOperation op, string bucketName,
+            TypedRedactor redactor)


I feel like passing in a specific IRedactor implementation will make testing/maintenance more difficult in the future.

jeffrymorris · 2026-09-08T23:45:52Z

+
+        public string? SystemDataString(string? value) =>
+            string.IsNullOrEmpty(value) ? value : SystemData(value).ToString();
+


These methods feel cringy/bloated. We already have the IRedactor interface and then these go around it.

Motivation ========== Review of the redaction change turned up four things worth acting on. CouchbaseException.ToJsonString() serialized the context with the default HTML-escaping encoder rather than the redaction-safe one, so its tags would come out as escape sequences. It turns out the method has no caller: it is internal, InternalsVisibleTo names only Couchbase.Test.Common and Couchbase.UnitTests, and the FIT performer its comment points at is a separate assembly via ProjectReference. Nothing in any repo calls it. RetryOrchestrator built its KeyValue context with UserDataString rather than the OperationKeyString helper the other two sites use, so it was a seventh site that could classify a SelectBucket key differently. Latent today - SelectBucket never reaches the orchestrator - but it is exactly the inconsistency the helper exists to prevent. Two ClusterNode lambdas captured a redactor local, which costs a display class on top of the delegate; GetClusterMap runs on every config poll. The classification test failed closed for a new field on a known context type, but only the six listed types were checked at all, so a whole new context type would have slipped past it. Modification ============ Deleted ToJsonString and InterfaceRuntimeTypeConverter, its only consumer. Routed the orchestrator's DocumentKey through OperationKeyString. Dropped the captured locals; the lambdas use _redactor directly, so they capture 'this' and cost the delegate alone. Added EveryErrorContextTypeIsAccountedFor, which scans the assembly for IErrorContext implementers and fails for any that is neither classified nor listed in NeedsNothing with a reason. That turns the two disclosed decisions - EventingFunctionErrorContext needs nothing, GenericErrorContext is NCBC-4300 - into asserted exclusions. The management redaction test now builds its context through the shared ErrorContextDrivers instead of its own copy of the mock harness. Results ======= 3021 pass on net8.0, zero failures, one more test than before. All five target frameworks build with no warnings. Confirmed the new scan bites: removing an entry from NeedsNothing fails it by name.

Motivation ========== Threading the redactor into the GetClusterMap and SelectBucket projectors cost them their static, so the compiler could no longer cache the two delegates and each call allocated instead. Dropping the captured local in the previous commit removed the display class but still left a delegate allocation per call, and GetClusterMap runs on every config poll. Modification ============ The redactor now comes back as a projector parameter, which is how the executor's state already reaches its lambda a few lines further down. Every projector is static again; the four that do not want the redactor discard it. Results ======= 3021 pass on net8.0, zero failures. All five target frameworks build with no warnings.

davidkelly requested a balanced review from Copilot September 4, 2026 22:02

Copilot started reviewing on behalf of davidkelly September 4, 2026 22:02 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread tests/Couchbase.UnitTests/Core/Exceptions/ErrorContextRedactionTests.cs Outdated

Comment thread src/Couchbase/Core/ClusterNode.cs Outdated

davidkelly and others added 2 commits September 4, 2026 17:42

davidkelly force-pushed the NCBC-4296 branch from 4490da2 to fc70048 Compare September 4, 2026 23:52

davidkelly requested a balanced review from Copilot September 4, 2026 23:57

Copilot started reviewing on behalf of davidkelly September 4, 2026 23:58 View session

Copilot AI reviewed Sep 5, 2026

View reviewed changes

davidkelly and others added 5 commits September 7, 2026 12:08

davidkelly marked this pull request as ready for review September 8, 2026 16:56

jeffrymorris reviewed Sep 8, 2026

View reviewed changes

davidkelly added 2 commits September 9, 2026 08:42

davidkelly marked this pull request as draft September 9, 2026 20:49

Navigation Menu

Uh oh!

NCBC-4296: Honour the configured redaction level in error contexts - #181

NCBC-4296: Honour the configured redaction level in error contexts#181
davidkelly wants to merge 10 commits into
masterfrom
NCBC-4296

davidkelly commented Sep 4, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

jeffrymorris Sep 8, 2026

Uh oh!

jeffrymorris Sep 8, 2026

Uh oh!

jeffrymorris Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		public string? SystemDataString(string? value) =>
		string.IsNullOrEmpty(value) ? value : SystemData(value).ToString();

Uh oh!

Conversation

davidkelly commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's classified as what

Judgement calls worth a reviewer's attention

Behaviour change

Known gap: the couchbase2:// path is not covered

Testing

Reading the commits

Follow-up recorded on NCBC-4297

Uh oh!

github-actions Bot commented Sep 4, 2026

FIT performer image

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

jeffrymorris Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

jeffrymorris Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

jeffrymorris Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davidkelly commented Sep 4, 2026 •

edited

Loading