eschutho opened a new pull request, #35800:
URL: https://github.com/apache/superset/pull/35800

   ### SUMMARY
   
   This PR fixes critical error handling issues in the report scheduling system 
that were causing:
   1. Confusing cascading errors with redacted "UPDATE" messages
   2. Silent error swallowing where failures weren't logged to the dashboard
   3. Recursive failures when database logging itself failed
   
   **Root Cause**: When a report schedule is deleted/modified by another 
process during execution, SQLAlchemy raises `StaleDataError`. The original code 
didn't handle this, leading to session rollback and subsequent 
`PendingRollbackError` when trying to access lazy-loaded attributes.
   
   **Changes**:
   1. **Handle StaleDataError gracefully** in `create_log()`: Catch the 
exception, rollback the session, and raise a clear error message that won't be 
redacted
   2. **Fix silent error swallowing** in `ReportSuccessState.next()`: Add 
missing `raise` statement so errors actually propagate
   3. **Prevent recursive logging failures**: Add exception guards around all 
`update_report_schedule_and_log()` calls to prevent infinite loops when logging 
itself fails
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A - Backend error handling improvement
   
   ### TESTING INSTRUCTIONS
   
   1. **Test StaleDataError handling**:
      - Start a long-running report execution (e.g., with a slow query)
      - Delete the report schedule while it's running
      - Verify: Error is logged to `report_execution_log` table with clear 
message
      - Verify: No cascade of `PendingRollbackError` exceptions
   
   2. **Test error propagation**:
      - Create a report with a chart that will fail (e.g., broken SQL)
      - Run the report
      - Verify: Error appears in dashboard logs and system logs
      - Verify: Error notification is sent to owners
   
   3. **Test normal execution**:
      - Run a successful report
      - Verify: No regressions in normal operation
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to