Refine structured error handler typing#2827
Refine structured error handler typing#2827abdulselamadillmohammed wants to merge 3 commits intosnowflakedb:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
Please add changelog entry in DESCRIPTION.md |
|
I have read the CLA Document and I hereby sign the CLA |
|
I’ve added a changelog entry in DESCRIPTION.md. Please let me know if anything else is needed or if you have any feedback. |
Behavioral change in
|
|
Thanks for calling this out. This behavior change was intentional. The goal of this PR was to align the structured error-handler path with the narrower contract used by the rest of the structured error machinery in Because of that, I changed I agree this is a behavioral change if a raw Also, while the typing changes in this PR were made on the sync connection/cursor I can update the PR description and changelog wording to make this behavior change more explicit, and I can also add async-path test coverage around the shared wrapper call sites if you’d like. |
|
@abdulselamadillmohammed how crucial is this change? Any breaking change will require a major version bump which we want to avoid for now |
|
Hi @sfc-gh-turbaszek , I’ve updated the logic to address the custom-handler and messages concerns you raised. Non-Snowflake exceptions no longer bypass the handler chain. I've updated errorhandler_wrapper_from_ready_exception to call hand_to_other_handler for all exceptions, so connection.messages and cursor.messages are populated correctly and custom handlers can intercept the error. If nothing handles the exception, the original exception instance is re-raised rather than constructing a new one. To keep the StructuredErrorHandler contract consistent, I am normalizing generic Python exceptions into an InterfaceError with a structured payload before passing them to handlers. This restores handler invocation and the messages updates, but it does still change what handlers see for generic exceptions: instead of the original Python exception class and args, handlers receive InterfaceError with a structured dict. I believe this is the cleanest way to preserve the structured handler contract while addressing the issues you pointed out, but please let me know if you’d prefer a closer restore of the old generic-exception behavior. |
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-3246741: Align structured error-handler typing and ready-exception handling #2790
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This change aligns the connector’s structured error-handling API with the contract already used in
errors.py.It introduces a shared structured payload type for error details and a shared protocol for error handlers, then applies that contract to the sync connection and cursor
errorhandlerattributes. It also narrows the structured error-handler flow to SnowflakeErrorsubclasses only.In addition, it updates
errorhandler_wrapper_from_ready_exception()so generic Python exceptions are re-raised directly instead of being routed into the dict-based structured handler path. This avoids the inconsistent case described in SNOW-3246741: Align structured error-handler typing and ready-exception handling #2790, where non-Snowflake exceptions could enter a handler flow that expects structured error details.The PR also adds unit tests covering:
- default structured error handling
- forwarding structured payloads to custom handlers
- normalization of ready
Errorinstances- direct re-raise behavior for generic exceptions
(Optional) PR for stored-proc connector: