Skip to content

Conversation

@marcorudolphflex
Copy link
Contributor

@marcorudolphflex marcorudolphflex commented Dec 16, 2025

Ensures that we don't do blind chaining when raising exceptions to have expressive errors logs in the GUI.
Same question as in #3093 :

  • do we want to do that in the pre-commit-hook? Or only CI?

Greptile Overview

Greptile Summary

This PR standardizes exception chaining across the codebase by ensuring that when exceptions are re-raised using raise NewError(...) from e, the original exception context is included in the error message using {e!s} formatting. This prevents "blind chaining" where the original error details are lost to the user.

Key changes:

  • New static analysis script (scripts/check_blind_chaining.py) using AST to detect blind exception chaining patterns
  • Integrated check into both pre-commit hooks and CI pipeline to enforce this pattern going forward
  • Updated 20+ exception handlers across components, plugins, and web API to include original error context
  • All error messages now follow pattern: "Description of new error: {original_exception!s}"

Impact:

  • Significantly improves debugging experience by preserving full error context
  • Automated enforcement prevents regression
  • Consistent error reporting across the entire codebase

Confidence Score: 4/5

  • This PR is safe to merge with minor formatting issues
  • The changes are mechanical and improve error handling without altering logic. The static analyzer is well-designed with appropriate safeguards. One minor formatting issue in sim_data.py with awkward spacing, and one potentially incorrect comment about pre-commit configuration that may confuse future maintainers
  • Pay attention to tidy3d/components/data/sim_data.py for the formatting issue with space before colon

Important Files Changed

File Analysis

Filename Score Overview
scripts/check_blind_chaining.py 5/5 New AST-based static analyzer that detects blind exception chaining patterns where original error context is lost
.github/workflows/tidy3d-python-client-tests.yml 5/5 Added CI step to run blind chaining check as part of code quality checks
.pre-commit-config.yaml 5/5 Added pre-commit hook for blind chaining check to catch issues early in development
tidy3d/components/base.py 5/5 Updated exception messages to include original error context using {exc!s} format
tidy3d/web/api/webapi.py 5/5 Standardized exception chaining in web API error handling to preserve original error details
tidy3d/components/data/sim_data.py 4/5 Added exception context to savemat error message; minor formatting issue with space before colon

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant PreCommit as Pre-commit Hook
    participant CI as CI Pipeline
    participant Script as check_blind_chaining.py
    participant Code as Codebase

    Dev->>Dev: Write code with raise...from
    Dev->>PreCommit: git commit
    PreCommit->>Script: Run blind chaining check
    Script->>Code: Parse Python files with AST
    Code-->>Script: Return AST nodes
    Script->>Script: Find raise...from patterns
    Script->>Script: Check if cause variable in message
    alt Blind chaining detected
        Script-->>PreCommit: Exit 1 (fail)
        PreCommit-->>Dev: Commit blocked
        Dev->>Dev: Add {e!s} to error message
    else No issues
        Script-->>PreCommit: Exit 0 (pass)
        PreCommit-->>Dev: Commit succeeds
    end
    
    Dev->>CI: Push to remote
    CI->>Script: Run blind chaining check
    Script->>Code: Parse Python files with AST
    Code-->>Script: Return AST nodes
    Script->>Script: Validate all raise...from patterns
    alt All patterns valid
        Script-->>CI: Exit 0 (pass)
        CI-->>Dev: Build passes
    else Invalid patterns found
        Script-->>CI: Exit 1 (fail)
        CI-->>Dev: Build fails
    end
Loading

@marcorudolphflex marcorudolphflex force-pushed the FXC-4466-standardize-chained-exception-messages-enforce-via-ci branch from 3f4e7d2 to 231f2e3 Compare December 16, 2025 13:48
@marcorudolphflex
Copy link
Contributor Author

@greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

23 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@marcorudolphflex marcorudolphflex force-pushed the FXC-4466-standardize-chained-exception-messages-enforce-via-ci branch from 231f2e3 to 4117571 Compare December 16, 2025 13:57
@marcorudolphflex marcorudolphflex marked this pull request as ready for review December 16, 2025 13:58
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

23 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

github-actions bot commented Dec 16, 2025

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

  • tidy3d/components/base.py (0.0%): Missing lines 211,221
  • tidy3d/plugins/autograd/functions.py (50.0%): Missing lines 54
  • tidy3d/plugins/dispersion/fit.py (50.0%): Missing lines 668
  • tidy3d/plugins/dispersion/web.py (0.0%): Missing lines 256,295
  • tidy3d/plugins/klayout/drc/drc.py (100%)
  • tidy3d/plugins/klayout/drc/results.py (100%)
  • tidy3d/web/api/webapi.py (0.0%): Missing lines 124,1674

Summary

  • Total: 13 lines
  • Missing: 8 lines
  • Coverage: 38%

tidy3d/components/base.py

Lines 207-215

  207             raise TypeError("Input must be a dict")
  208         try:
  209             type_value = obj[TYPE_TAG_STR]
  210         except KeyError as exc:
! 211             raise ValueError(f'Missing "{TYPE_TAG_STR}" in data: {exc!s}') from exc
  212         if not isinstance(type_value, str) or not type_value:
  213             raise ValueError(f'Invalid "{TYPE_TAG_STR}" value: {type_value!r}')
  214         return type_value

Lines 217-225

  217     def _get_registered_class(cls, type_value: str) -> type[Tidy3dBaseModel]:
  218         try:
  219             return TYPE_TO_CLASS_MAP[type_value]
  220         except KeyError as exc:
! 221             raise ValueError(f"Unknown type: {type_value}: {exc!s}") from exc
  222 
  223     @classmethod
  224     def _should_dispatch_to(cls, target_cls: type[Tidy3dBaseModel]) -> bool:
  225         """Return True if ``cls`` allows auto-dispatch to ``target_cls``."""

tidy3d/plugins/autograd/functions.py

Lines 50-58

  50         if not isinstance(ax, int):
  51             try:
  52                 ax = int(ax)
  53             except Exception as e:
! 54                 raise TypeError(f"Axis {ax!r} could not be converted to an integer: {e!s}") from e
  55 
  56         if not -ndim <= ax < ndim:
  57             raise ValueError(f"Invalid axis {ax} for {kind} with ndim {ndim}.")
  58         return ax + ndim if ax < 0 else ax

tidy3d/plugins/dispersion/fit.py

Lines 664-672

  664 
  665         try:
  666             resp.raise_for_status()
  667         except Exception as e:
! 668             raise WebError(
  669                 f"Connection to the website failed. Please provide a valid URL: {e!s}"
  670             ) from e
  671 
  672         data_url = list(

tidy3d/plugins/dispersion/web.py

Lines 252-260

  252             ssl_verify = False
  253             resp = requests.get(f"{url_server}/health", verify=ssl_verify)
  254             resp.raise_for_status()
  255         except Exception as e:
! 256             raise WebError(f"Connection to the server failed. Please try again: {e!s}") from e
  257 
  258         return get_headers(), ssl_verify
  259 
  260     def run(self) -> tuple[PoleResidue, float]:

Lines 291-299

  291                         "Fitter failed due to timeout. Try to decrease the number of tries or "
  292                         "inner iterations, or to relax the RMS tolerance."
  293                     )
  294                 )
! 295                 raise Tidy3dError(f"{msg}: {e!s}") from e
  296 
  297             raise WebError(
  298                 f"Fitter failed. Try again, tune the parameters, or contact us for more help: {e!s}"
  299             ) from e

tidy3d/web/api/webapi.py

Lines 120-128

  120         batch_detail = batch.detail()
  121         status = batch_detail.status.lower()
  122     except Exception as e:
  123         log.error(f"Could not retrieve batch details for '{resource_id}': {e}")
! 124         raise WebError(f"Failed to retrieve status for batch '{resource_id}': {e!s}") from e
  125 
  126     if status not in ERROR_STATES:
  127         return

Lines 1670-1678

  1670         console = get_logging_console()
  1671         console.log("Authentication configured successfully!")
  1672     except (WebError, HTTPError) as e:
  1673         url = "https://docs.flexcompute.com/projects/tidy3d/en/latest/index.html"
! 1674         raise WebError(
  1675             f"{e!s}\n\n"
  1676             "It looks like the Tidy3D Python interface is not configured with your "
  1677             "unique API key. "
  1678             "To get your API key, sign into 'https://tidy3d.simulation.cloud' and copy it "

@marcorudolphflex marcorudolphflex force-pushed the FXC-4466-standardize-chained-exception-messages-enforce-via-ci branch from 4117571 to fd4299c Compare December 16, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants