Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/tidy3d-python-client-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -981,6 +1003,7 @@ jobs:
- determine-test-scope
- local-tests
- remote-tests
- check-error-blind-chaining
- lint
- mypy
- verify-schema-change
Expand All @@ -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: |
Expand Down
8 changes: 8 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
160 changes: 160 additions & 0 deletions scripts/check_blind_chaining.py
Original file line number Diff line number Diff line change
@@ -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 <new_exc> from <cause>` where `<cause>` 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 <name>` 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:]))
2 changes: 1 addition & 1 deletion scripts/make_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions tidy3d/components/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)):
Expand Down
2 changes: 1 addition & 1 deletion tidy3d/components/base_sim/data/sim_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion tidy3d/components/data/data_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tidy3d/components/data/sim_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tidy3d/components/field_projection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tidy3d/components/geometry/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion tidy3d/components/microwave/path_integrals/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
2 changes: 1 addition & 1 deletion tidy3d/components/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion tidy3d/components/viz/plot_sim_3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tidy3d/plugins/autograd/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}.")
Expand Down Expand Up @@ -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)


Expand Down
2 changes: 1 addition & 1 deletion tidy3d/plugins/autograd/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions tidy3d/plugins/dispersion/fit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down Expand Up @@ -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)
Expand Down
Loading