Add client-side rich error interceptor by miruuna · Pull Request #995 · DUNE-DAQ/drunc

miruuna · 2026-08-04T16:19:20Z

Description

Fixes issue #973

Implements a client-side interceptor for gRPC rich error handling.

The interceptor is documented here.

Type of change

List of required branches from other repositories

None

Change log

added client interceptor RichErrorClientInterceptor in src/drunc/utils/grpc_utils.py
attached client interceptor to channel in SessionManager and ProcessManager
updated tests

Suggested manual testing checklist

Run pytest.

Developer checklist

Prior to marking this as "Ready for Review"

Tests ran on: WHAT HOSTNAME from release RELEASE_NAME

Unit tests - some tests can't be ran on the CI. This is documented. If this PR checks a feature that can't be tested with CI, this has been marked appropriately.

Integration tests - the daqsystemtest_integtest_bundle requires a lot of resources, and connections to the EHN1 infrastructure. Check the cross referenced list if you can't run these. The developer needs to run at least the .

Unit tests (pytest --marker) passed
- With relevant marker
- Without marker
Integration tests passed
- Only daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py
- Full daqsystemtest_integtest_bundle.sh
Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows
Drunc integration tests pass (./scripts/drunc_integtest_bundle.sh)

Final checklist prior to marking this as "Ready for Review"

Code is clearly commented.
New unit tests have been added, or is documented in # ISSUE NUMBER
A suitable reviewer has been chosen from this list.

Reviewer checklist

This branch has been rebased with develop prior to testing.
Suggested manual tests show changes.
CI workflows fails documented (if present)
Integration tests passed (on either np0x or IC HEP clusters)
- Use the following guidelines to determine which of the integration tests you need to run
  - You do not need to run any integration tests if
    - Code changes are not associated with src/
    - PR changes only affect docstrings
    - In this case, be sure to validate any suggested manual testing.
  - Run only the minimum integration test as daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py if
    - PR changes only affect a few log entries
    - PR changes are small, and do not have a large impact on the workflow (use carefully)
  - Otherwise run the full integration test bundle as daqsystemtest_integtest_bundle.sh
- What to do if the integration tests fail?
  - Only concern yourself if failures related to drunc are in the log files
  - If non-drunc failure appears:
    - Validate failure in fresh working area
    - Contact Pawel if unsure
If you have ran the full integration test bundle, leave a comment on the PR stating
- Which host the integration tests have ran on
- [Optional] A copy of the test summary
Drunc integration tests pass (scripts/drunc_integtest_bundle.sh)

Once the above boxes are checked, the PR(s) can be merged following the steps below.

Prior to merging

Choose one of the following an complete all substeps

Changes only affect the Run Control, are in a single repository, and do not affect the end user.
- Changes are documented in docstrings and code comments
- Wiki has been updated if architectural or endpoint changes
Otherwise
- Workflow changes demonstrated in the Change Log (if necessary)
- Wiki has been updated (if necessary)
- #dunedaq-integration Slack channel notified (see below)

Once completed, the reviewer can merge the PR.

Notification message for a Slack channel

Note - this should be to #dunedaq-integration for general workflow that isn't during a release candidate period, and to #daq-release-prep otherwise.

For an single merge that changes the user workflow

The CCM WG has an isolated PR ready to merge that affects user workflows. The PR is:

_URL_

I will leave time for any comments, otherwise will merge these at the end of the work day _Insert your time zone_.

For co-ordinated merge

The CCM WG has a set of co-ordinated merges ready to merge. The PRs are:

_URL_

_URL_


I will leave time for any comments, otherwise will merge these at the end of the day.

…-client-interceptor

PawelPlesniak · 2026-08-06T11:29:48Z

Integration tests passed on np04-srv-028 using NFD_DEV_260806_A9

+++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++ SUMMARY ++++++++++++++++++++
+++++++++++++++++++++++++++++++++++++++++++++++++

Thu Aug  6 12:23:10 PM CEST 2026
Log file is: /tmp/pytest-of-pplesnia/dunedaq_integtest_bundle_20260806113629.log

⮕ Running daqsystemtest/3ru_1df_multirun_test.py ⬅
======================== 6 passed ✅ in 260.14s (0:04:20) =========================
⮕ Running daqsystemtest/3ru_3df_multirun_test.py ⬅
======================== 6 passed ✅ in 249.65s (0:04:09) =========================
⮕ Running daqsystemtest/disabled_tpg_test.py ⬅
========================= 3 passed ✅ in 62.79s (0:01:02) =========================
⮕ Running daqsystemtest/example_system_test.py ⬅
=================== 4 failed ❌, 8 passed ✅ in 512.93s (0:08:32) ====================
⮕ Running daqsystemtest/fake_data_producer_test.py ⬅
======================== 6 passed ✅ in 240.63s (0:04:00) =========================
⮕ Running daqsystemtest/long_window_readout_test.py ⬅
============================== 1 skipped 🟡 in 1.24s ==============================
⮕ Running daqsystemtest/minimal_system_quick_test.py ⬅
============================== 4 passed ✅ in 53.84s ==============================
⮕ Running daqsystemtest/readout_type_scan_test.py ⬅
======================== 33 passed ✅ in 602.20s (0:10:02) ========================
⮕ Running daqsystemtest/sample_ehn1_multihost_test.py ⬅
======================== 4 skipped 🟡 in 82.86s (0:01:22) =========================
⮕ Running daqsystemtest/small_footprint_quick_test.py ⬅
============================== 3 passed ✅ in 55.57s ==============================
⮕ Running daqsystemtest/tpg_state_collection_test.py ⬅
======================== 5 passed ✅ in 108.09s (0:01:48) =========================
⮕ Running daqsystemtest/tpreplay_test.py ⬅
======================== 6 passed ✅ in 133.67s (0:02:13) =========================
⮕ Running daqsystemtest/tpstream_writing_test.py ⬅
======================== 4 passed ✅ in 107.76s (0:01:47) =========================
⮕ Running daqsystemtest/trigger_bitwords_test.py ⬅
======================== 18 passed ✅ in 310.01s (0:05:10) ========================

The intended behaviour demonstrated in the unit tests has passed too, thank you!

PawelPlesniak · 2026-08-06T11:30:18Z

Will leave time for @jamesturner246 to take a look before merging

jamesturner246 · 2026-08-07T16:09:31Z

There are ~~a few places~~ one place where try has been deleted, but the proceeding block has not been un-indented, causing issues.. e.g. process_manager_driver boot, line 211, the boot rpc is only actually called if session_dal.opmon_uri.type == "file".

I'll need to look a bit more in-depth into this to find them all, but this one definitely shouldn't be merged yet. Surprised the tests even passed.

jamesturner246

Forgot to add review flag.

ED: actually I think it's just the one place. Feel free to override my review once fixed.

jamesturner246 · 2026-08-07T16:16:12Z

Otherwise looks really good. Nice neat and elegant 👍 👍

jamesturner246

LGTM! 👍

miruuna · 2026-08-07T17:00:16Z

Forgot to add review flag.

ED: actually I think it's just the one place. Feel free to override my review once fixed.

Thanks for spotting that @jamesturner246! Not sure how I missed that and strange how the tests passed

miruuna · 2026-08-07T17:17:18Z

@PawelPlesniak I'd like to add some more tests to use a real Process Manager instead of a concrete one, in a similar way to what I did to the session manager tests. That's to make sure we don't miss any errors on the client side.

I'll do that early next week if you can wait until then to merge. Thanks.

PawelPlesniak · 2026-08-09T08:42:10Z

Thanks @miruuna! There is progress being made on integration tests running in mutli-process running modes (e.g. drunc-process-manager and drunc-process-manager-shell running in the same integration test), so depending how you'd like to do it, it can be done either as a unit test or as an integration test. We can disucss next week if you'd like

miruuna · 2026-08-13T06:50:01Z

Thanks @miruuna! There is progress being made on integration tests running in mutli-process running modes (e.g. drunc-process-manager and drunc-process-manager-shell running in the same integration test), so depending how you'd like to do it, it can be done either as a unit test or as an integration test. We can disucss next week if you'd like

For the rich error handling it's probably enough to update the unit tests. The tests in test_process_manager_rich_errors.py and test_session_manager_rich_errors.py are more integration style as they use a real gRPC server with the real server interceptor and a real client stub with the client interceptor. I've updated the tests to check that extract_grpc_rich_errors is called to verify that the client interceptor works (extract_grpc_rich_error is only called within the interceptor wrapper).

…-client-interceptor

PawelPlesniak · 2026-08-14T10:38:30Z

Thanks Miruna!
The following tests passed on np04-srv-028 using NFD_DEV_260814_A9

Unit tests
Integration tests:


+++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++ SUMMARY ++++++++++++++++++++
+++++++++++++++++++++++++++++++++++++++++++++++++

Fri Aug 14 12:19:44 PM CEST 2026
Log file is: /tmp/pytest-of-pplesnia/dunedaq_integtest_bundle_20260814113751.log

⮕ Running daqsystemtest/3ru_1df_multirun_test.py ⬅
======================== 6 passed ✅ in 246.92s (0:04:06) =========================
⮕ Running daqsystemtest/3ru_3df_multirun_test.py ⬅
======================== 6 passed ✅ in 250.09s (0:04:10) =========================
⮕ Running daqsystemtest/disabled_tpg_test.py ⬅
========================= 3 passed ✅ in 60.76s (0:01:00) =========================
⮕ Running daqsystemtest/example_system_test.py ⬅
======================== 12 passed ✅ in 241.42s (0:04:01) ========================
⮕ Running daqsystemtest/fake_data_producer_test.py ⬅
======================== 6 passed ✅ in 242.90s (0:04:02) =========================
⮕ Running daqsystemtest/long_window_readout_test.py ⬅
============================== 1 skipped 🟡 in 1.31s ==============================
⮕ Running daqsystemtest/minimal_system_quick_test.py ⬅
============================== 4 passed ✅ in 53.39s ==============================
⮕ Running daqsystemtest/readout_type_scan_test.py ⬅
======================== 33 passed ✅ in 593.76s (0:09:53) ========================
⮕ Running daqsystemtest/sample_ehn1_multihost_test.py ⬅
======================== 4 skipped 🟡 in 89.99s (0:01:29) =========================
⮕ Running daqsystemtest/small_footprint_quick_test.py ⬅
============================== 3 passed ✅ in 55.55s ==============================
⮕ Running daqsystemtest/tpg_state_collection_test.py ⬅
======================== 5 passed ✅ in 108.20s (0:01:48) =========================
⮕ Running daqsystemtest/tpreplay_test.py ⬅
======================== 6 passed ✅ in 127.10s (0:02:07) =========================
⮕ Running daqsystemtest/tpstream_writing_test.py ⬅
======================== 4 passed ✅ in 110.07s (0:01:50) =========================
⮕ Running daqsystemtest/trigger_bitwords_test.py ⬅
======================== 18 passed ✅ in 313.37s (0:05:13) ========================

The drunc specific integration tests failed, which is associated with the merge of #990. This has been addressed in #1008

miruuna added 7 commits July 22, 2026 18:08

Implement ClientInterceptor in SessionManager

a9ce32b

Implement client interceptor in process manager

8f19f7c

Merge branch 'develop' into miruuna/973-feature-grpc-rich-error---add…

757b0c8

…-client-interceptor

Update session manager tests

fb18860

Update markdown with client interceptor

70572c4

Merge branch 'develop' into miruuna/973-feature-grpc-rich-error---add…

1c904c0

…-client-interceptor

Update tests docstrings

f21e36d

miruuna linked an issue Aug 4, 2026 that may be closed by this pull request

[Feature]: gRPC rich error - Add Client Interceptor #973

Closed

miruuna requested review from PawelPlesniak and jamesturner246 August 4, 2026 16:31

PawelPlesniak added 2 commits August 6, 2026 11:32

Merge branch 'develop' into miruuna/973-feature-grpc-rich-error---add…

30e5428

…-client-interceptor

Moving documentation to docs path

8c9bec7

PawelPlesniak approved these changes Aug 6, 2026

View reviewed changes

jamesturner246 requested changes Aug 7, 2026

View reviewed changes

Fix indentation in process manager driver

4ffb73d

jamesturner246 approved these changes Aug 7, 2026

View reviewed changes

Add more checks for RichErrorClientInterceptor

d7f88f5

miruuna requested a review from PawelPlesniak August 13, 2026 06:50

miruuna and others added 2 commits August 13, 2026 09:01

Add missing request.token.CopyFrom() in log_on_server

00add64

Merge branch 'develop' into miruuna/973-feature-grpc-rich-error---add…

1843c56

…-client-interceptor

PawelPlesniak merged commit ca40cf4 into develop Aug 14, 2026
5 checks passed

PawelPlesniak deleted the miruuna/973-feature-grpc-rich-error---add-client-interceptor branch August 14, 2026 10:38

Navigation Menu

Uh oh!

Add client-side rich error interceptor - #995

Add client-side rich error interceptor#995
PawelPlesniak merged 13 commits into
developfrom
miruuna/973-feature-grpc-rich-error---add-client-interceptor

miruuna commented Aug 4, 2026 •

edited by PawelPlesniak

Loading

Uh oh!

PawelPlesniak commented Aug 6, 2026

Uh oh!

PawelPlesniak commented Aug 6, 2026

Uh oh!

jamesturner246 commented Aug 7, 2026 •

edited

Loading

Uh oh!

jamesturner246 left a comment •

edited

Loading

Uh oh!

jamesturner246 commented Aug 7, 2026

Uh oh!

jamesturner246 left a comment

Uh oh!

miruuna commented Aug 7, 2026 •

edited

Loading

Uh oh!

miruuna commented Aug 7, 2026

Uh oh!

PawelPlesniak commented Aug 9, 2026

Uh oh!

miruuna commented Aug 13, 2026

Uh oh!

PawelPlesniak commented Aug 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

miruuna commented Aug 4, 2026 • edited by PawelPlesniak Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

List of required branches from other repositories

Change log

Suggested manual testing checklist

Developer checklist

Prior to marking this as "Ready for Review"

Final checklist prior to marking this as "Ready for Review"

Reviewer checklist

Prior to merging

Notification message for a Slack channel

For an single merge that changes the user workflow

For co-ordinated merge

Uh oh!

PawelPlesniak commented Aug 6, 2026

Uh oh!

PawelPlesniak commented Aug 6, 2026

Uh oh!

jamesturner246 commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamesturner246 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesturner246 commented Aug 7, 2026

Uh oh!

jamesturner246 left a comment

Choose a reason for hiding this comment

Uh oh!

miruuna commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miruuna commented Aug 7, 2026

Uh oh!

PawelPlesniak commented Aug 9, 2026

Uh oh!

miruuna commented Aug 13, 2026

Uh oh!

PawelPlesniak commented Aug 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

miruuna commented Aug 4, 2026 •

edited by PawelPlesniak

Loading

jamesturner246 commented Aug 7, 2026 •

edited

Loading

jamesturner246 left a comment •

edited

Loading

miruuna commented Aug 7, 2026 •

edited

Loading