Skip to content

Conversation

@shaheeramjad
Copy link

Description

This PR adds exception chaining (raise ... from) throughout the Python SDK to preserve error context and improve debuggability. When exceptions are re-raised without chaining, the original traceback context is lost, making it difficult to identify root causes of failures.

Changes Made

  1. CloudSQLEnrichmentHandler (transforms/enrichment_handlers/cloudsql.py)

    • Added from e to exception re-raises in _execute_query() method
    • Preserves original database exception context when database operations fail
  2. Process Utilities (utils/processes.py)

    • Added exception chaining for OSError and CalledProcessError in call(), check_call(), and check_output() functions
    • Preserves original subprocess exception context when external executables fail
  3. Core Transforms (transforms/core.py)

    • Improved exception logging to include exc_info=True when failure callbacks fail
    • Preserves full traceback in logs for better debugging

Benefits

  • Better Debugging: Developers can now see the full error chain when exceptions occur
  • Faster Issue Resolution: Root causes are immediately visible in stack traces
  • Python Best Practice: Follows PEP 3134 (Exception Chaining and Embedded Tracebacks)
  • Improved Error Messages: More informative stack traces for production debugging

Example

Before:

except Exception as e:
    raise RuntimeError(f"Database operation failed: {e}")

After:

except Exception as e:
    raise RuntimeError(f"Database operation failed: {e}") from e

Now when this fails, developers see:

RuntimeError: Database operation failed: connection timeout
The above exception was the direct cause of the following exception:
ConnectionTimeoutError: Failed to connect within 30 seconds
  [full original traceback here]

Testing

  • Syntax validation passed
  • No linter errors introduced
  • Changes preserve existing behavior (only adds exception context)
  • Backward compatible (no breaking changes)

Checklist


Related


- Add 'from e' to exception re-raises in CloudSQLEnrichmentHandler
- Add exception chaining in processes.py for OSError and CalledProcessError
- Improve logging in core.py to preserve traceback context

This improves debuggability by preserving the full exception chain,
following Python PEP 3134 best practices.

Fixes apache#37422
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @shaheeramjad, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the debuggability of the Python SDK by implementing explicit exception chaining (raise ... from) across several critical components. By linking re-raised exceptions to their original causes, the changes ensure that developers have access to complete error tracebacks, which is essential for quickly identifying and resolving the root causes of failures. This enhancement aligns with Python's best practices for error handling and contributes to more robust and informative error reporting.

Highlights

  • CloudSQL Enrichment Handler: Implemented exception chaining in the _execute_query method within CloudSQLEnrichmentHandler to preserve the original database exception context when operations fail.
  • Process Utilities: Added exception chaining for OSError and subprocess.CalledProcessError in the call(), check_call(), and check_output() functions, ensuring the original subprocess exception context is retained.
  • Core Transforms Logging: Enhanced exception logging in exception_handling_wrapper_do_fn_custom_process by including exc_info=True when on_failure_callback itself fails, providing a full traceback in the logs.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions
Copy link
Contributor

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

@shaheeramjad
Copy link
Author

assign set of reviewers

@github-actions
Copy link
Contributor

Assigning reviewers:

R: @claudevdm for label python.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@shaheeramjad
Copy link
Author

assign to next reviewer

@shaheeramjad
Copy link
Author

HI @damccorm,
Just a gentle ping for review when you have time. Thanks!

except Exception as e:
transaction.rollback()
raise RuntimeError(f"Database operation failed: {e}")
raise RuntimeError(f"Database operation failed: {e}") from e
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're chaining the exception, do we still need to include the exception in the RuntimeError message?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same question exists below in this file as well

Copy link
Author

Choose a reason for hiding this comment

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

With from e, the cause is in cause and the traceback, so we don’t need to include the exception in the message.
I kept it in because in many environments only the top-level exception message is logged or shown (e.g. in alerts or dashboards). In those cases, "Database operation failed" alone doesn’t tell you the real error, while f"Database operation failed: {e}" stays useful. It’s also a common Python pattern to add context and still include the original error in the wrapper message.
If you prefer to rely on the chain and keep messages minimal, I’m happy to change to e.g. raise RuntimeError("Database operation failed") from e and remove {e} from the message. Happy to follow whatever convention you prefer for this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants