Skip to content

Add get_tls_connection_error_elaboration to allow clients to get more descriptive errors - #14

Merged
alexdubois-ni merged 1 commit into
mainfrom
users/adubois/addElaborationExtractionMechanismForClients
Aug 25, 2026
Merged

alexdubois-ni merged 1 commit into
mainfrom
users/adubois/addElaborationExtractionMechanismForClients

Conversation

@alexdubois-ni

@alexdubois-ni alexdubois-ni commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Add get_tls_connection_error_elaboration to allow clients to get more descriptive errors

What does this Pull Request accomplish?

Adds a new get_tls_connection_error_elaboration that returns a more useful error elaboration for diagnosing gRPC unavailable errors when NI-TLS is in the equation.

Why should this Pull Request be merged?

As of today, users will get a very generic unavailable status from gRPC. We want to re-interpret this into something more useful, but clients will need to call this new get_tls_connection_error_elaboration explicitly themselves to accomplish this.

When clients prompt for this elaboration with this function, they will have this error essentially toggled off/on depending on whether the nitlsconfig python package flagged that it was a first-class created grpc device channel from our nitlsconfig python package. If it is not a nitlsconfig created channel (e.g. a standard insecure or secure channel created by a user), then they will proceed with normal errors as they deem it.

What testing has been done?

Unit tests.

…place grpc::unavailable error descriptions with an NI one that mentions that the reasoning maybe NI-TLS related
@alexdubois-ni
alexdubois-ni requested a balanced review from Copilot August 24, 2026 22:54
@ni-github-admins

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

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

Adds opt-in TLS connection error elaboration for channels created by NI-TLS configuration.

Changes:

  • Tags created gRPC channels for later identification.
  • Adds and exports connection-error elaboration.
  • Refactors audit tagging and adds unit tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/nitlsconfig/channel_tag.py Implements channel tagging and lookup.
src/nitlsconfig/errors.py Adds TLS connection error elaboration.
src/nitlsconfig/grpc_channel.py Tags created channels.
src/nitlsconfig/audit.py Uses shared channel-tag utilities.
src/nitlsconfig/__init__.py Exports the new API.
tests/unit/test_channel_tag.py Tests channel tagging.
tests/unit/test_errors.py Tests elaboration behavior.
tests/unit/test_grpc_channel.py Verifies created channels are tagged.
tests/unit/test_audit.py Updates audit tests for the refactor.

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

@alexdubois-ni
alexdubois-ni requested a review from ckoellin August 24, 2026 22:58
@github-actions

Copy link
Copy Markdown

Test Results

 12 files  ± 0   12 suites  ±0   17s ⏱️ +2s
 72 tests + 6   72 ✅ + 6  0 💤 ±0  0 ❌ ±0 
864 runs  +72  864 ✅ +72  0 💤 ±0  0 ❌ ±0 

Results for commit c9cdcd0. ± Comparison against base commit 64a21d5.

This pull request removes 1 and adds 7 tests. Note that renamed tests count towards both.
tests.unit.test_audit ‑ test___channel_rejects_attributes___tagging_does_not_raise
tests.unit.test_channel_tag ‑ test___channel_carrying_a_foreign_attribute___is_not_recognized_as_ours
tests.unit.test_channel_tag ‑ test___channel_rejects_attributes___tagging_does_not_raise
tests.unit.test_channel_tag ‑ test___tagged_channel___reports_its_target
tests.unit.test_channel_tag ‑ test___untagged_channel___is_not_recognized_as_ours
tests.unit.test_errors ‑ test___channel_we_created___elaborates_on_connection_errors
tests.unit.test_errors ‑ test___channel_we_did_not_create___does_not_elaborate
tests.unit.test_grpc_channel ‑ test_created_channels_are_tagged_as_ours

@pkthong pkthong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good.

Comment thread src/nitlsconfig/errors.py
@alexdubois-ni
alexdubois-ni merged commit e60cdcc into main Aug 25, 2026
31 checks passed
@alexdubois-ni
alexdubois-ni deleted the users/adubois/addElaborationExtractionMechanismForClients branch August 25, 2026 16:40
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