diff --git a/.github/workflows/tidy3d-python-client-tests.yml b/.github/workflows/tidy3d-python-client-tests.yml index 7c3ab7c71f..69e6c6af6c 100644 --- a/.github/workflows/tidy3d-python-client-tests.yml +++ b/.github/workflows/tidy3d-python-client-tests.yml @@ -224,6 +224,28 @@ jobs: echo "extras_integration_tests=$extras_integration_tests" echo "test_type=$test_type" + check-error-blind-chaining: + name: check blind-chaining of error messages + needs: determine-test-scope + if: needs.determine-test-scope.outputs.code_quality_tests == 'true' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 1 + submodules: false + persist-credentials: false + + - name: set-python-3.10 + uses: actions/setup-python@v4 + with: + python-version: '3.10' + + - name: Run blind-chaining test + run: | + set -euo pipefail + python scripts/check_blind_chaining.py + lint: needs: determine-test-scope if: needs.determine-test-scope.outputs.code_quality_tests == 'true' @@ -981,6 +1003,7 @@ jobs: - determine-test-scope - local-tests - remote-tests + - check-error-blind-chaining - lint - mypy - verify-schema-change @@ -993,6 +1016,12 @@ jobs: - extras-integration-tests runs-on: ubuntu-latest steps: + - name: check-error-blind-chaining + if: ${{ needs.determine-test-scope.outputs.code_quality_tests == 'true' && needs.check-error-blind-chaining.result != 'success' && needs.check-error-blind-chaining.result != 'skipped' }} + run: | + echo "❌ Detected blind-chained error messages." + exit 1 + - name: check-linting-result if: ${{ needs.determine-test-scope.outputs.code_quality_tests == 'true' && needs.lint.result != 'success' && needs.lint.result != 'skipped' }} run: | diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fce5cbc0cc..2fd6b60d88 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,6 +11,14 @@ repos: stages: [pre-commit] - id: ruff-format stages: [pre-commit] + - repo: local + hooks: + - id: blind-chaining-check + name: blind-chaining-check + entry: python scripts/check_blind_chaining.py + language: system + types: [python] + pass_filenames: false - repo: https://github.com/alessandrojcm/commitlint-pre-commit-hook rev: v9.22.0 hooks: diff --git a/scripts/check_blind_chaining.py b/scripts/check_blind_chaining.py new file mode 100644 index 0000000000..5c5316dd57 --- /dev/null +++ b/scripts/check_blind_chaining.py @@ -0,0 +1,160 @@ +"""Detect blind exception chaining that hides original errors. + +Usage: + python scripts/check_blind_chaining.py [paths ...] +""" + +from __future__ import annotations + +import ast +import sys +from collections.abc import Iterable +from pathlib import Path + +SKIP_COMMENT = "# noqa: BC" +ALLOWLIST_DIRS = ( + Path("tidy3d/web/cli/develop"), + Path("tidy3d/packaging"), +) +ALLOWLIST_PATHS = ( + Path("tidy3d/packaging.py"), + Path("tidy3d/updater.py"), +) + + +def contains_name(node: ast.AST | None, target: str) -> bool: + """Return True if any ``ast.Name`` inside ``node`` matches ``target``.""" + + if node is None: + return False + return any(isinstance(child, ast.Name) and child.id == target for child in ast.walk(node)) + + +def iter_python_files(paths: Iterable[Path]) -> Iterable[Path]: + """Yield Python files under the provided paths, respecting skips.""" + + for root in paths: + if root.is_file() and root.suffix == ".py": + yield root + continue + if not root.is_dir(): + continue + yield from root.rglob("*.py") + + +def _primary_message_expr(exc: ast.AST) -> ast.AST | None: + """ + Heuristic: identify the expression that most serializers/GUI layers display. + + - For `SomeError("msg", ...)`: first positional arg. + - For `SomeError(message="msg")` / `detail=...`: preferred keyword. + - Otherwise: None (unknown). + """ + if not isinstance(exc, ast.Call): + return None + + if exc.args: + return exc.args[0] + return None + + +def find_blind_chaining(path: Path) -> list[tuple[Path, int, int, str]]: + """ + Find `raise from ` where `` is *not* referenced in the + user-visible message expression (first positional arg or message-like kwarg). + + Returns: (path, lineno, col_offset, cause_name) + """ + errors: list[tuple[Path, int, int, str]] = [] + try: + src = path.read_text(encoding="utf-8") + tree = ast.parse(src, filename=str(path)) + except SyntaxError: + return errors + + lines = src.splitlines() + + for node in ast.walk(tree): + if not isinstance(node, ast.Raise): + continue + + # Handles `raise from e` (node.exc is None) safely. + if node.exc is None: + continue + + # Ignore `raise X from None` (intentional suppression). + if isinstance(node.cause, ast.Constant) and node.cause.value is None: + continue + + # Only enforce for simple `from ` patterns (e.g., `except ... as e:`). + if not isinstance(node.cause, ast.Name): + continue + cause_name = node.cause.id + + # Avoid noisy false positives for `raise existing_exc from e`. + if isinstance(node.exc, ast.Name): + continue + + # Prefer checking the “primary message” expression if we can identify it. + msg_expr = _primary_message_expr(node.exc) + + if msg_expr is not None: + ok = contains_name(msg_expr, cause_name) + else: + # Fallback: at least require the cause to be used somewhere in the exc expression. + ok = contains_name(node.exc, cause_name) + + if not ok: + lineno = getattr(node, "lineno", 1) + col = getattr(node, "col_offset", 0) + if lineno - 1 < len(lines): + if SKIP_COMMENT in lines[lineno - 1]: + continue + errors.append((path, lineno, col, cause_name)) + + return errors + + +def is_allowlisted(path: Path) -> bool: + """Return True if ``path`` resides in an allowlisted directory.""" + + resolved_path = path.resolve() + if any(resolved_path == allow_path.resolve() for allow_path in ALLOWLIST_PATHS): + return True + for allow_dir in ALLOWLIST_DIRS: + if resolved_path.is_relative_to(allow_dir.resolve()): + return True + return False + + +def main(argv: list[str]) -> int: + paths = [Path(arg) for arg in argv] if argv else [Path("tidy3d")] + existing_paths = [path for path in paths if path.exists()] + if not existing_paths: + existing_paths = [Path(".")] + + failures: list[tuple[Path, int, int, str]] = [] + for file_path in iter_python_files(existing_paths): + failures.extend(find_blind_chaining(file_path)) + + filtered_failures = [ + (path, lineno, cause_name) + for path, lineno, _, cause_name in failures + if not is_allowlisted(path) + ] + + if filtered_failures: + print("Blind exception chaining detected (missing original cause in raised message):") + for path, lineno, cause_name in sorted(filtered_failures): + print( + f" {path}:{lineno} cause variable '{cause_name}' not referenced in raised exception" + ) + print(f"Add '{SKIP_COMMENT}' to the raise line to suppress intentionally.") + return 1 + + print("No blind exception chaining instances found.") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/scripts/make_script.py b/scripts/make_script.py index cbd061f095..f50d24d303 100644 --- a/scripts/make_script.py +++ b/scripts/make_script.py @@ -88,7 +88,7 @@ def main(args): except subprocess.CalledProcessError as exc: raise RuntimeError( "Ruff formatting failed. Your script might not be compatible with make_script.py. " - "This could be due to unsupported features like CustomMedium." + f"This could be due to unsupported features like CustomMedium.\n{exc!s}" ) from exc finally: # remove the temporary file diff --git a/tidy3d/components/base.py b/tidy3d/components/base.py index cc7a8d91c5..9f074ff3a2 100644 --- a/tidy3d/components/base.py +++ b/tidy3d/components/base.py @@ -208,7 +208,7 @@ def _get_type_value(cls, obj: dict[str, Any]) -> str: try: type_value = obj[TYPE_TAG_STR] except KeyError as exc: - raise ValueError(f'Missing "{TYPE_TAG_STR}" in data') from exc + raise ValueError(f'Missing "{TYPE_TAG_STR}" in data: {exc!s}') from exc if not isinstance(type_value, str) or not type_value: raise ValueError(f'Invalid "{TYPE_TAG_STR}" value: {type_value!r}') return type_value @@ -218,7 +218,7 @@ def _get_registered_class(cls, type_value: str) -> type[Tidy3dBaseModel]: try: return TYPE_TO_CLASS_MAP[type_value] except KeyError as exc: - raise ValueError(f"Unknown type: {type_value}") from exc + raise ValueError(f"Unknown type: {type_value}: {exc!s}") from exc @classmethod def _should_dispatch_to(cls, target_cls: type[Tidy3dBaseModel]) -> bool: @@ -387,7 +387,8 @@ def updated_copy( raise AttributeError( f"Could not field field '{field_name}' in the sub-component `path`. " f"Found fields of '{tuple(self.__fields__.keys())}'. " - "Please double check the `path` passed to `.updated_copy()`." + "Please double check the `path` passed to `.updated_copy()`: " + f"{e!s}" ) from e if isinstance(sub_component, (list, tuple)): diff --git a/tidy3d/components/base_sim/data/sim_data.py b/tidy3d/components/base_sim/data/sim_data.py index 86e752cc55..6368955f9d 100644 --- a/tidy3d/components/base_sim/data/sim_data.py +++ b/tidy3d/components/base_sim/data/sim_data.py @@ -67,7 +67,7 @@ def data_monitors_match_sim(cls, values): except Tidy3dKeyError as exc: raise DataError( f"Data with monitor name '{monitor_name}' supplied " - f"but not found in the original '{sim.type}'." + f"but not found in the original '{sim.type}': {exc!s}" ) from exc return values diff --git a/tidy3d/components/data/data_array.py b/tidy3d/components/data/data_array.py index 7c7cbae61f..dee77c0ec4 100644 --- a/tidy3d/components/data/data_array.py +++ b/tidy3d/components/data/data_array.py @@ -520,7 +520,8 @@ def _with_updated_data(self, data: np.ndarray, coords: dict[str, Any]) -> DataAr raise ValueError( "Couldn't reshape the supplied 'data' to update 'DataArray'. The provided data was " f"of shape {data.shape} and tried to reshape to {new_shape}. If you encounter this " - "error please raise an issue on the tidy3d github repository with the context." + "error please raise an issue on the tidy3d github repository with the context: " + f"{e!s}" ) from e # broadcast data to repeat data along the selected dimensions to match mask diff --git a/tidy3d/components/data/sim_data.py b/tidy3d/components/data/sim_data.py index fdb0c6b552..3bffb1ffbd 100644 --- a/tidy3d/components/data/sim_data.py +++ b/tidy3d/components/data/sim_data.py @@ -1382,5 +1382,6 @@ def to_mat_file(self, fname: PathLike, **kwargs: Any) -> None: savemat(fname, modified_sim_dict, **kwargs) except Exception as e: raise ValueError( - "Could not save supplied 'SimulationData' to file. As this is an experimental feature, we may not be able to support the contents of your dataset. If you receive this error, please feel free to raise an issue on our front end repository so we can investigate." + "Could not save supplied 'SimulationData' to file. As this is an experimental feature, we may not be able to support the contents of your dataset. If you receive this error, please feel free to raise an issue on our front end repository so we can investigate " + f"{e!s}" ) from e diff --git a/tidy3d/components/field_projection.py b/tidy3d/components/field_projection.py index 4346a85d50..d952eb230e 100644 --- a/tidy3d/components/field_projection.py +++ b/tidy3d/components/field_projection.py @@ -423,7 +423,7 @@ def _far_fields_for_surface( currents_f = currents.sel(f=frequency) except Exception as e: raise SetupError( - f"Frequency {frequency} not found in fields for monitor '{surface.monitor.name}'." + f"Frequency {frequency} not found in fields for monitor '{surface.monitor.name}': {e!s}" ) from e idx_w, idx_uv = surface.monitor.pop_axis((0, 1, 2), axis=surface.axis) diff --git a/tidy3d/components/geometry/base.py b/tidy3d/components/geometry/base.py index 9a2fc61401..21754d5d84 100644 --- a/tidy3d/components/geometry/base.py +++ b/tidy3d/components/geometry/base.py @@ -1473,7 +1473,7 @@ def to_gds_file( except ImportError as e: raise Tidy3dImportError( "Python module 'gdstk' not found. To export geometries to .gds " - "files, please install it." + f"files, please install it: {e!s}" ) from e library = gdstk.Library() diff --git a/tidy3d/components/microwave/path_integrals/factory.py b/tidy3d/components/microwave/path_integrals/factory.py index 9100dcc206..f926e0f469 100644 --- a/tidy3d/components/microwave/path_integrals/factory.py +++ b/tidy3d/components/microwave/path_integrals/factory.py @@ -148,6 +148,7 @@ def make_path_integrals( raise SetupError( f"Failed to construct path integrals for the mode with index {idx} " "from the impedance specification. " - "Please create a github issue so that the problem can be investigated." + "Please create a github issue so that the problem can be investigated: " + f"{e!s}" ) from e return (tuple(v_integrals), tuple(i_integrals)) diff --git a/tidy3d/components/structure.py b/tidy3d/components/structure.py index 2d99c55559..9b07900cab 100644 --- a/tidy3d/components/structure.py +++ b/tidy3d/components/structure.py @@ -632,7 +632,7 @@ def to_gds_file( except ImportError as e: raise Tidy3dImportError( "Python module 'gdstk' not found. To export geometries to .gds " - "files, please install it." + f"files, please install it: {e!s}" ) from e cell = library.new_cell(gds_cell_name) self.to_gds( diff --git a/tidy3d/components/viz/plot_sim_3d.py b/tidy3d/components/viz/plot_sim_3d.py index 7111309446..c89402cdaa 100644 --- a/tidy3d/components/viz/plot_sim_3d.py +++ b/tidy3d/components/viz/plot_sim_3d.py @@ -68,7 +68,7 @@ def plot_sim_3d(sim, width=800, height=800, is_gz_base64=False) -> None: except ImportError as e: raise SetupError( "3D plotting requires ipython to be installed " - "and the code to be running on a jupyter notebook." + f"and the code to be running on a jupyter notebook: {e!s}" ) from e from base64 import b64encode diff --git a/tidy3d/plugins/autograd/functions.py b/tidy3d/plugins/autograd/functions.py index 578a1efc79..922be34a8a 100644 --- a/tidy3d/plugins/autograd/functions.py +++ b/tidy3d/plugins/autograd/functions.py @@ -51,7 +51,7 @@ def _normalize_single_axis(ax: SupportsInt, ndim: int, kind: str) -> int: try: ax = int(ax) except Exception as e: - raise TypeError(f"Axis {ax!r} could not be converted to an integer.") from e + raise TypeError(f"Axis {ax!r} could not be converted to an integer: {e!s}") from e if not -ndim <= ax < ndim: raise ValueError(f"Invalid axis {ax} for {kind} with ndim {ndim}.") @@ -209,7 +209,7 @@ def _get_pad_indices( try: indices = onp.pad(onp.arange(n), (pad_left, pad_right), mode=mode) except ValueError as error: - raise ValueError(f"Unsupported padding mode: {mode}") from error + raise ValueError(f"Unsupported padding mode: {mode}: {error!s}") from error return numpy_module.asarray(indices, dtype=int) diff --git a/tidy3d/plugins/autograd/utilities.py b/tidy3d/plugins/autograd/utilities.py index 7a7b5f83a8..f312937898 100644 --- a/tidy3d/plugins/autograd/utilities.py +++ b/tidy3d/plugins/autograd/utilities.py @@ -232,7 +232,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: raise Tidy3dError( "An objective function's return value must be a scalar " "but got an array with shape " - f"{getattr(result, 'shape', 'N/A')}." + f"{getattr(result, 'shape', 'N/A')}: {e!s}" ) from e # Ensure the result is real diff --git a/tidy3d/plugins/dispersion/fit.py b/tidy3d/plugins/dispersion/fit.py index 56a5aaef78..729f389952 100644 --- a/tidy3d/plugins/dispersion/fit.py +++ b/tidy3d/plugins/dispersion/fit.py @@ -603,7 +603,9 @@ def _validate_url_load(data_load: list) -> None: try: _ = [float(x) for x in row] except Exception as e: - raise ValidationError("Invalid URL. Float data cannot be recognized.") from e + raise ValidationError( + f"Invalid URL. Float data cannot be recognized: {e!s}" + ) from e if has_k > 1: raise ValidationError("Invalid URL. Too many k labels.") @@ -663,7 +665,9 @@ def from_url( try: resp.raise_for_status() except Exception as e: - raise WebError("Connection to the website failed. Please provide a valid URL.") from e + raise WebError( + f"Connection to the website failed. Please provide a valid URL: {e!s}" + ) from e data_url = list( csv.reader(codecs.iterdecode(resp.iter_lines(), "utf-8"), delimiter=delimiter) diff --git a/tidy3d/plugins/dispersion/web.py b/tidy3d/plugins/dispersion/web.py index 69a2666c06..f78c2cccc7 100644 --- a/tidy3d/plugins/dispersion/web.py +++ b/tidy3d/plugins/dispersion/web.py @@ -253,7 +253,7 @@ def _setup_server(url_server: str) -> tuple[dict[str, str], bool]: resp = requests.get(f"{url_server}/health", verify=ssl_verify) resp.raise_for_status() except Exception as e: - raise WebError("Connection to the server failed. Please try again.") from e + raise WebError(f"Connection to the server failed. Please try again: {e!s}") from e return get_headers(), ssl_verify @@ -292,10 +292,10 @@ def run(self) -> tuple[PoleResidue, float]: "inner iterations, or to relax the RMS tolerance." ) ) - raise Tidy3dError(msg) from e + raise Tidy3dError(f"{msg}: {e!s}") from e raise WebError( - "Fitter failed. Try again, tune the parameters, or contact us for more help." + f"Fitter failed. Try again, tune the parameters, or contact us for more help: {e!s}" ) from e run_result = resp.json() diff --git a/tidy3d/plugins/klayout/drc/drc.py b/tidy3d/plugins/klayout/drc/drc.py index 010081e46b..ae14c70322 100644 --- a/tidy3d/plugins/klayout/drc/drc.py +++ b/tidy3d/plugins/klayout/drc/drc.py @@ -98,7 +98,9 @@ def _validate_drc_args_stringable(cls, v: Any) -> dict[str, str]: try: v = {str(k): str(v) for k, v in v.items()} except Exception as e: - raise ValidationError("Could not coerce keys and values of drc_args to strings.") from e + raise ValidationError( + f"Could not coerce keys and values of drc_args to strings: {e!s}" + ) from e return v @validator("drc_args") diff --git a/tidy3d/plugins/klayout/drc/results.py b/tidy3d/plugins/klayout/drc/results.py index 5093524b50..4276bbbb12 100644 --- a/tidy3d/plugins/klayout/drc/results.py +++ b/tidy3d/plugins/klayout/drc/results.py @@ -413,9 +413,11 @@ def violations_from_file( try: xmltree = ET.parse(resultsfile) except FileNotFoundError as err: - raise FileError(f"DRC result file not found: '{resultsfile}'.") from err + raise FileError(f"DRC result file not found: '{resultsfile}': {err!s}") from err except ET.ParseError as err: - raise ET.ParseError(f"Invalid XML format in DRC result file: '{resultsfile}'.") from err + raise ET.ParseError( + f"Invalid XML format in DRC result file: '{resultsfile}': {err!s}" + ) from err # Initialize violations dict with all the categories violations = {} diff --git a/tidy3d/plugins/smatrix/component_modelers/terminal.py b/tidy3d/plugins/smatrix/component_modelers/terminal.py index e9be3e1596..4a226f1d5e 100644 --- a/tidy3d/plugins/smatrix/component_modelers/terminal.py +++ b/tidy3d/plugins/smatrix/component_modelers/terminal.py @@ -511,7 +511,8 @@ def _finalized_radiation_monitors(self) -> tuple[DirectivityMonitor, ...]: raise ValueError( "Automatic construction of radiation monitors failed. " "Please address the reason or provide a tuple of DirectivityMonitor " - "objects to the 'radiation_monitors' parameter." + "objects to the 'radiation_monitors' parameter: " + f"{e!s}" ) from e else: # DirectivityMonitor - use as-is diff --git a/tidy3d/web/api/mode.py b/tidy3d/web/api/mode.py index 1927ac8b9f..8047dddcf8 100644 --- a/tidy3d/web/api/mode.py +++ b/tidy3d/web/api/mode.py @@ -630,7 +630,8 @@ def get_result( except Exception as e: raise WebError( "Failed to download the simulation data file from the server. " - "Please confirm that the task was successfully run." + "Please confirm that the task was successfully run: " + f"{e!s}" ) from e data = ModeSolverData.from_hdf5(to_file) diff --git a/tidy3d/web/api/webapi.py b/tidy3d/web/api/webapi.py index c51ccc2e6a..17a4baa80b 100644 --- a/tidy3d/web/api/webapi.py +++ b/tidy3d/web/api/webapi.py @@ -121,7 +121,7 @@ def _batch_detail_error(resource_id: str) -> Optional[WebError]: status = batch_detail.status.lower() except Exception as e: log.error(f"Could not retrieve batch details for '{resource_id}': {e}") - raise WebError(f"Failed to retrieve status for batch '{resource_id}'.") from e + raise WebError(f"Failed to retrieve status for batch '{resource_id}': {e!s}") from e if status not in ERROR_STATES: return @@ -141,7 +141,7 @@ def _batch_detail_error(resource_id: str) -> Optional[WebError]: ) except Exception as e: raise WebError( - "One or more subtasks failed validation. Failed to parse validation errors." + f"One or more subtasks failed validation. Failed to parse validation errors: {e!s}" ) from e raise WebError(full_error_msg) @@ -1671,10 +1671,9 @@ def test() -> None: console.log("Authentication configured successfully!") except (WebError, HTTPError) as e: url = "https://docs.flexcompute.com/projects/tidy3d/en/latest/index.html" - msg = ( - str(e) - + "\n\n" - + "It looks like the Tidy3D Python interface is not configured with your " + raise WebError( + f"{e!s}\n\n" + "It looks like the Tidy3D Python interface is not configured with your " "unique API key. " "To get your API key, sign into 'https://tidy3d.simulation.cloud' and copy it " "from your 'Account' page. Then you can configure tidy3d through command line " @@ -1683,5 +1682,4 @@ def test() -> None: "'.tidy3d/config' (windows) with content like: \n\n" "apikey = 'XXX' \n\nHere XXX is your API key copied from your account page within quotes.\n\n" f"For details, check the instructions at {url}." - ) - raise WebError(msg) from e + ) from e diff --git a/tidy3d/web/core/task_core.py b/tidy3d/web/core/task_core.py index f1e4ca8c7d..b2a4827e81 100644 --- a/tidy3d/web/core/task_core.py +++ b/tidy3d/web/core/task_core.py @@ -322,7 +322,8 @@ def get_data_hdf5( except Exception as e: raise WebError( "Failed to download the data file from the server. " - "Please confirm that the task completed successfully." + "Please confirm that the task completed successfully: " + f"{e!s}" ) from e return file @@ -820,7 +821,8 @@ def validate_post_upload(self, parent_tasks: Optional[list[str]] = None) -> None except Exception as e: raise ValidationError( "The parent task must be a 'VolumeMesher' task which has been successfully " - "run and is associated to the same 'HeatChargeSimulation' as provided here." + "run and is associated to the same 'HeatChargeSimulation' as provided here: " + f"{e!s}" ) from e except Exception as e: