Skip to content

Conversation

Pouyanpi
Copy link
Collaborator

@Pouyanpi Pouyanpi commented Aug 5, 2025

Description

Improve error handling in parallel output rails during streaming to properly represent internal errors

Changes include:

  • Modified runtime to return internal_error status with error messages
  • Updated SSE streaming to emit proper error events for internal failures
  • Added comprehensive tests for various internal error scenarios
  • Ensure internal errors are properly propagated through the streaming pipeline

…output rails

Improve error handling in parallel output rails during streaming to properly
  surface internal errors instead of silently failing. Changes include:

  - Modified runtime to return internal_error status with error messages
  - Updated SSE streaming to emit proper error events for internal failures
  - Added comprehensive tests for various internal error scenarios
  - Ensure internal errors are properly propagated through the streaming pipeline
@Pouyanpi Pouyanpi force-pushed the fix/internal-error-sse-streaming branch from 0947d20 to 0291570 Compare August 5, 2025 17:36
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.56%. Comparing base (dd1ce5f) to head (de6efe3).

Files with missing lines Patch % Lines
nemoguardrails/colang/v1_0/runtime/runtime.py 73.33% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1324      +/-   ##
===========================================
+ Coverage    70.52%   70.56%   +0.03%     
===========================================
  Files          161      161              
  Lines        16267    16284      +17     
===========================================
+ Hits         11473    11491      +18     
+ Misses        4794     4793       -1     
Flag Coverage Δ
python 70.56% <84.61%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
nemoguardrails/rails/llm/llmrails.py 89.94% <100.00%> (+0.12%) ⬆️
nemoguardrails/colang/v1_0/runtime/runtime.py 84.61% <73.33%> (-0.25%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Pouyanpi Pouyanpi requested a review from tgasser-nv August 5, 2025 18:40
@Pouyanpi Pouyanpi self-assigned this Aug 5, 2025
@Pouyanpi Pouyanpi added this to the v0.15.0 milestone Aug 5, 2025
@Pouyanpi Pouyanpi added the bug Something isn't working label Aug 5, 2025
@Pouyanpi
Copy link
Collaborator Author

Pouyanpi commented Aug 5, 2025

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review. ✅ Project coverage is 70.56%. Comparing base (dd1ce5f) to head (de6efe3).

this is not correct, if we open the report we see all lines are covered.

Copy link
Collaborator

@tgasser-nv tgasser-nv left a comment

Choose a reason for hiding this comment

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

LGTM, CI tests are passing now

@Pouyanpi Pouyanpi merged commit 44d7cda into develop Aug 5, 2025
38 of 39 checks passed
@Pouyanpi Pouyanpi deleted the fix/internal-error-sse-streaming branch August 5, 2025 18:54
Pouyanpi added a commit that referenced this pull request Aug 5, 2025
#1324)

Improve error handling in parallel output rails during streaming to properly surface internal errors instead of silently failing.

Changes include:
  - Modified runtime to return internal_error status with error messages
  - Updated SSE streaming to emit proper error events for internal failures
  - Added comprehensive tests for various internal error scenarios
  - Ensure internal errors are properly propagated through the streaming pipeline
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants