From 61f45e0b0f74e340312be1e1005bc362aca132ee Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Mon, 12 Jan 2026 12:17:36 -0500 Subject: [PATCH 1/4] Make exceptions more idiomatic. Fix #19. * Based on a union of all the `result` error types from the WIT, generate more specific exception classes. * Generate patches to make componentize-py-generated routines raise those exceptions. * Apply those monkeypatches at `fastly_compute` import time. * Move `remap_wit_error()` to its new home right next to the monkeypatcher which should be its exclusive caller. * Teach makefile to run the code generator at the right times. --- .gitignore | 7 +- Makefile | 14 +- fastly_compute/__init__.py | 6 + fastly_compute/exceptions/__init__.py | 29 ++ fastly_compute/wit_patching/__init__.py | 1 + .../decorators.py} | 26 +- fastly_compute/wsgi.py | 17 +- pyproject.toml | 4 + scripts/generate_patches/__init__.py | 25 ++ scripts/generate_patches/__main__.py | 3 + scripts/generate_patches/generation.py | 215 ++++++++++ scripts/generate_patches/utils.py | 38 ++ scripts/generate_patches/wit.py | 370 ++++++++++++++++++ tests/test_nice_exceptions.py | 2 +- 14 files changed, 715 insertions(+), 42 deletions(-) create mode 100644 fastly_compute/exceptions/__init__.py create mode 100644 fastly_compute/wit_patching/__init__.py rename fastly_compute/{exceptions.py => wit_patching/decorators.py} (75%) create mode 100755 scripts/generate_patches/__init__.py create mode 100755 scripts/generate_patches/__main__.py create mode 100755 scripts/generate_patches/generation.py create mode 100644 scripts/generate_patches/utils.py create mode 100644 scripts/generate_patches/wit.py diff --git a/.gitignore b/.gitignore index 9edbf5e..026b456 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,13 @@ # Python/uv artifacts .venv/ *.egg-info +__pycache__ +# Generated code /stubs/ -__pycache__ +/fastly_compute/exceptions/* +!/fastly_compute/exceptions/__init__.py +/fastly_compute/wit_patching/patches.py # Build artifacts /build/ @@ -12,4 +16,3 @@ bin/ # Rust target/ *.so -target/ diff --git a/Makefile b/Makefile index d4db6d3..6b9f08a 100644 --- a/Makefile +++ b/Makefile @@ -51,12 +51,18 @@ $(STUBS_DIR): $(COMPUTE_WIT) uv run componentize-py -d wit --world-module wit_world -w $(TARGET_WORLD) bindings $(STUBS_DIR) # Build our composed wasm using fastly-compute-py build -$(BUILD_DIR)/%.composed.wasm: wit/viceroy.wit wit/deps/fastly/compute.wit fastly_compute/wsgi.py | $(BUILD_DIR) $(STUBS_DIR) +$(BUILD_DIR)/%.composed.wasm: wit/viceroy.wit wit/deps/fastly/compute.wit fastly_compute/wsgi.py fastly_compute/wit_patching/patches.py | $(BUILD_DIR) $(STUBS_DIR) @echo "Building $* example with fastly-compute-py..." @test -d $(EXAMPLES_DIR)/$* || (echo "Error: Example directory $(EXAMPLES_DIR)/$* not found" && exit 1) @test -f $(EXAMPLES_DIR)/$*/$*.py || (echo "Error: Example file $(EXAMPLES_DIR)/$*/$*.py not found" && exit 1) cd $(EXAMPLES_DIR)/$* && $(FASTLY_COMPUTE_PY) build --output ../../$@ +# The script that writes the exceptions and the patches always rewrites +# everything, so we can depend on the mod date of only 1 file. We choose +# patches.py, because its name doesn't depend on the WIT contents. +fastly_compute/wit_patching/patches.py: scripts/generate_patches/*.py $(COMPUTE_WIT) + uv run python -m scripts.generate_patches + # Create build directory $(BUILD_DIR): mkdir -p $(BUILD_DIR) @@ -82,10 +88,12 @@ list-examples: # Clean build artifacts clean: rm -rf $(BUILD_DIR) $(STUBS_DIR) + rm -f fastly_compute/wit_patching/patches.py + cd fastly_compute/exceptions && rm -rf acl http_body http_req kv_store types cd crates/fastly-compute-py && cargo clean # Development tools -lint: | $(STUBS_DIR) +lint: fastly_compute/wit_patching/patches.py | $(STUBS_DIR) @echo "Checking version synchronization..." uv run python scripts/check_version_sync.py @echo "Linting Python code..." @@ -94,7 +102,7 @@ lint: | $(STUBS_DIR) @echo "Linting Rust code..." cd crates/fastly-compute-py && cargo clippy -- -D warnings -lint-fix: +lint-fix: fastly_compute/wit_patching/patches.py @echo "Fixing Python code..." uv run --extra dev ruff check --fix . @echo "Fixing Rust code..." diff --git a/fastly_compute/__init__.py b/fastly_compute/__init__.py index 84534fc..5d7d2e4 100644 --- a/fastly_compute/__init__.py +++ b/fastly_compute/__init__.py @@ -5,3 +5,9 @@ # Testing utilities are available but not imported by default # Users can import them explicitly: from fastly_compute.testing import ViceroyTestBase + +from fastly_compute.wit_patching.patches import patch + +# Before anything from the fastly_compute package is used, do our monkeypatching +# to make the WIT-generated code act more Pythonically: +patch() diff --git a/fastly_compute/exceptions/__init__.py b/fastly_compute/exceptions/__init__.py new file mode 100644 index 0000000..5a6e37b --- /dev/null +++ b/fastly_compute/exceptions/__init__.py @@ -0,0 +1,29 @@ +"""Top-level exceptions emitted by the Fastly API""" + + +class FastlyError(Exception): + """Abstract base class for all errors raised by Fastly APIs + + This allows catching all errors emanating from Fastly APIs at once. + """ + + +class UnexpectedFastlyError(FastlyError): + """An error arising from a Fastly API but of an unanticipated kind, such + that we merely package up the low-level error and send it along. + + Any of these encountered in the wild means we neglected to keep our Python + wrappers up to date with the WIT. + """ + + def __init__(self, error_value: object): + """Construct. + + :arg error_value: The ``value`` attr of the raised ``Err`` + """ + self.value = error_value + + +# I went with the exact verbatim names of the error cases, not appending "Error" +# to the ends of the ones that didn't have it to make them strictly conform to +# Python conventions. "except HttpInvalid" reads fine to me. diff --git a/fastly_compute/wit_patching/__init__.py b/fastly_compute/wit_patching/__init__.py new file mode 100644 index 0000000..9c163f6 --- /dev/null +++ b/fastly_compute/wit_patching/__init__.py @@ -0,0 +1 @@ +"""Monkeypatches (and supporting machinery) which make WIT behavior more Pythonic""" diff --git a/fastly_compute/exceptions.py b/fastly_compute/wit_patching/decorators.py similarity index 75% rename from fastly_compute/exceptions.py rename to fastly_compute/wit_patching/decorators.py index dc829a1..8dbcb1d 100644 --- a/fastly_compute/exceptions.py +++ b/fastly_compute/wit_patching/decorators.py @@ -1,4 +1,4 @@ -"""Top-level exceptions emitted by the Fastly API""" +"""Decorators used in runtime patching""" from collections.abc import Callable, Mapping from enum import Enum @@ -7,31 +7,9 @@ from wit_world.imports.types import Err +from fastly_compute.exceptions import FastlyError, UnexpectedFastlyError -class FastlyError(Exception): - """Abstract base class for all errors raised by Fastly APIs - This allows catching all errors emanating from Fastly APIs at once. - """ - - -class UnexpectedFastlyError(FastlyError): - """An error arising from a Fastly API but of an unanticipated kind, such - that we merely package up the low-level error and send it along. - - Any of these encountered in the wild means we neglected to keep our Python - wrappers up to date with the WIT. - """ - - def __init__(self, error_value: object): - """Construct. - - :arg error_value: The ``value`` attr of the raised ``Err`` - """ - self.value = error_value - - -# TODO: Move to somewhere more private once it becomes clear where. def remap_wit_errors( idiomatic_exceptions: Mapping[Any, type[FastlyError]] | None = None, ) -> Callable: diff --git a/fastly_compute/wsgi.py b/fastly_compute/wsgi.py index ba6e294..da3834b 100644 --- a/fastly_compute/wsgi.py +++ b/fastly_compute/wsgi.py @@ -22,7 +22,8 @@ next_request, ) from wit_world.imports.http_resp import send_downstream -from wit_world.imports.types import Err, Error_CannotRead + +from fastly_compute.exceptions.types.error import CannotRead def serve_wsgi_request( @@ -222,17 +223,9 @@ def handle(self, request: Any, body: Any) -> None: pending_request = next_request(options) try: result = await_request(pending_request) - except Err as exc: - # TODO: Improve error design so we can catch only the exceptions - # we're really interested in, per Python's idiom. Rather than - # carting around a Result type that's Union[Ok[T], Err[E]], we - # should probably return T xor raise E. - if isinstance(exc.value, Error_CannotRead): - # There were no more requests within the timeout. - break - else: - # Something went wrong. - raise + except CannotRead: + # There were no more requests within the timeout. + break else: if not result: break diff --git a/pyproject.toml b/pyproject.toml index 5ec3cbb..62517d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,6 +55,7 @@ ignore = [ "UP031", # % string formatting, minimally disruptive for stdlib-based HTML templating "D415", # Don't require punctuation at the end of a docstring summary: sometimes they are noun phrases, not sentences. "D205", # This spuriously complains about line-wrapped single-sentence docstring summaries. Sometimes 80 chars isn't enough. + "D107", # Sometimes there's nothing non-obvious to say about a constructor. ] [tool.ruff.format] @@ -70,6 +71,8 @@ convention = "google" [tool.ruff.lint.per-file-ignores] "tests/*" = ["D"] "examples/*" = ["D"] +# What can one say about __main__? +"__main__.py" = ["D100"] [build-system] requires = ["maturin>=1.0,<2.0"] @@ -99,6 +102,7 @@ project-includes = [ "fastly_compute/**/*.py", "tests/**/*.py", "examples/**/*.py", + "scripts", ] project-excludes = [ # CLI wrapper imports from native extension that doesn't exist until built diff --git a/scripts/generate_patches/__init__.py b/scripts/generate_patches/__init__.py new file mode 100755 index 0000000..52849ea --- /dev/null +++ b/scripts/generate_patches/__init__.py @@ -0,0 +1,25 @@ +"""A generator of Python code which improves upon the ergonomics and +idiomaticness of the WIT-derived code generated by componentize-py + +This runs at SDK-build time; customers don't run it. + +Generation is informed by the WIT. We considered informing it from +componentize-py's generated Python, but that turned out to be infeasibly lossy. +For example, for exception mapping, componentize-py's dataclasses and enum +members (which go into the .value properties of Err exceptions and correspond to +the more specific exceptions we'd like to raise) had nothing relating them to +methods except a brittle "Raises" docstring. And that docstring began to fail +with more complex error types, e.g. ones which are option<>s: +`wit_world.types.Err(wit_world.imports.Optional[Any])` is what comes out of +`result<_, option>`. + +On the surface, one downside of our chosen method is that we assume a stable +relationship of the WIT to componentize-py's generated stubs. But that must +remain stable, or callers would break. A true though slight downside is that +there are quite a few reimplemented bits of knowledge: WIT-interface-to-module +mappings, case conventions, and so on. Finally, there's the error-proneness of +JSON-struct-chasing and the need for wasm-tools. However, none of this is needed +beyond SDK build time, DRY code in the WIT processing should break spectacularly +if at all, and tests that touch any of the customer API should quickly reveal +such breakage. +""" diff --git a/scripts/generate_patches/__main__.py b/scripts/generate_patches/__main__.py new file mode 100755 index 0000000..8a9a637 --- /dev/null +++ b/scripts/generate_patches/__main__.py @@ -0,0 +1,3 @@ +from .generation import generate + +generate() diff --git a/scripts/generate_patches/generation.py b/scripts/generate_patches/generation.py new file mode 100755 index 0000000..039f5a7 --- /dev/null +++ b/scripts/generate_patches/generation.py @@ -0,0 +1,215 @@ +"""Top level of the code-generation that makes exception-raising more idiomatic +in Fastly SDK routines + +Handles high-level logic and writing to the filesystem. +""" + +import json +from collections import defaultdict +from collections.abc import Iterable, Mapping +from pathlib import Path +from subprocess import check_output + +from .wit import Function, NullType, Type, Wit + +WIT_DIR = "wit" + + +def exception_code_tree(error_types: Iterable[Type]) -> Mapping[str, Mapping[str, str]]: + """Generate Python exception classes we can map error types to. + + Inherit names and docstrings from the WIT. Create a common superclass for + each type so you can catch the whole smear if you like. + + :arg error_types: An iterable of unique types used as error arms of + ``result``s + + :return: A dict of package names pointing to module names pointing to + contained code. For example, acl (from the interface name) -> acl_error.py + (from the enum name) -> class AclError(FastlyError)... + """ + # interface name -> module name -> code chunks: + code = defaultdict(lambda: defaultdict(str)) + + for error_type in error_types: + package = error_type.py_package() + module = error_type.py_module() + ".py" + + # Create package's empty __init__.py if not already there: + code[package]["__init__.py"] + + if not code[package][module]: + code[package][module] += ( + "from fastly_compute.exceptions import FastlyError\n\n\n" + ) + + # Common superclass for exceptions based on the enum or variant's + # members. Or the raised exception itself for records. + code[package][module] += ( + f"""class {error_type.py_exception_name()}(FastlyError):\n""" + f''' """{error_type.docstring_or_pass()}"""\n\n\n''' + ) + # Insert enum or variant cases. + for case in error_type.cases(): + code[package][module] += ( + f"""class {case.py_exception_name()}({error_type.py_exception_name()}):\n""" + f''' """{case.docstring_or_pass()}"""\n\n\n''' + ) + return code + + +def mappings_code_tree( + error_types: Iterable[Type], functions_to_patch: Iterable[Function] +) -> dict[str, dict[str, str]]: + """Generate code which makes componentize-py-generated routines raise more + specific, idiomatically shaped exceptions. + + Map componentize-py's Err values to specific exceptions. Generate + monkeypatches that wrap componentize-py's generated Python routines to raise + them. + """ + code = ( + """# This file is automatically generated by generate_patches.py.\n""" + """# It is not intended for manual editing.\n""" + '''"""Monkeypatches which wrap the routines generated by componentize-py to make\n''' + '''them raise more specific exceptions, not just Err."""\n\n''' + ) + + # Collect info: + mappings = set() + imports = set() + for error_type in error_types: + # Get where it is found in wit_world. Use shallow imports to avoid collisions. + imports.add(error_type.wit_module_path()) + imports.add(error_type.py_module_path()) + if error_type.has_cases(): + # We need only add the cases; it doesn't make sense in WIT to return + # the Enum or Variant itself in a result. + for case in error_type.cases(): + mappings.add( + ( + case.wit_path(), + error_type.py_module_path(), + case.py_exception_name(), + ) + ) + else: + mappings.add( + ( + error_type.wit_path(), + error_type.py_module_path(), + error_type.py_exception_name(), + ) + ) + + # Collect import paths for the functions themselves: + for func in functions_to_patch: + imports.add(func.wit_module_path()) + + # Do templating: + code += ( + "from .decorators import remap_wit_errors\n" # Linter: don't join these lines. + "import fastly_compute.exceptions\n" + ) + for import_ in sorted(imports): + code += f"import {import_}\n" + code += "\n\n" + + code += "MAPPINGS = {\n" + for wit_path, py_module_path, py_exception_name in sorted(mappings): + code += f" {wit_path}: {py_module_path}.{py_exception_name},\n" + code += " type(None): fastly_compute.exceptions.FastlyError,\n" + code += "}\n\n" + + code += ''' +did_patch = False + + +def patch(): + """Apply patches if they haven't already been applied.""" + + global did_patch + if did_patch: + # This test shouldn't be needed, but it avoids double-wrapping the + # routines if somehow patch() did get called twice. + return + did_patch = True + +''' + + for func in functions_to_patch: + func_path = func.wit_path() + code += f" {func_path} = remap_wit_errors(MAPPINGS)({func_path})\n" + + # TODO: Make affordance for manually adding ergonomic getter properties, + # __str__s, etc. to exception classes. + + # TODO: Maybe automatically improve the docstring of each method to list the + # exceptions it raises. + + return {"wit_patching": {"patches.py": code}} + + +def write_files(tree: Mapping[str, Mapping[str, str]], base_folder: Path): + """Create filesystem artifacts mirroring a nested dict representing folders, + then files, then file contents. + + Overwrite files that are mentioned in ``tree``, but don't delete anything else. + """ + for folder, files in tree.items(): + folder_path = base_folder / folder + folder_path.mkdir(parents=True, exist_ok=True) + for file, contents in files.items(): + file_path = folder_path / file + file_path.write_text(contents) + + +def generate(): + """Generate idiomatic exceptions and monkeypatches to get WIT functions to + raise them. + + Currently, this handles only ``result`` error types that are variants, + enums, records, or the unit type. It doesn't handle options or primitives, + but it would be straightfoward to expand as necessary. The only interesting + decision to make when expanding is what kind of exception to raise: for + enums, variants, and records, we generate an exception class corresponding + to each case and raise that. But you can't raise a plain int. Maybe raise a + generic FastlyError? We throw a NotImplementedError during generation if we + do encounter something unsupported. + """ + wit_text = check_output(["wasm-tools", "component", "wit", WIT_DIR, "--json"]) + wit_json = json.loads(wit_text) + wit = Wit(wit_json) + + # A dict preserves order, for comprehensibility and determinism of generated code: + exceptions_to_generate: dict[Type, bool] = {} + functions_to_patch = [] + + # Hunt through our whole fastly-compute package to find the result error + # types we return. Each inspires the generation of one exception class (in + # the case of records) or more (in the case of variants or enums). + for interface in wit.fastly_compute_package().interfaces(): + for function in interface.functions(): + if error_type := function.error_type_of_returned_result(): + if not isinstance(error_type, NullType): + # Null errors (result) are handled by a static + # entry mapping it to FastlyError. + + # We don't need to go any deeper than the top-level type of + # the result's error. That represents the whole universe of + # Err values the componentize-py-generated bits may raise. + # Those values are what we will promote to exceptions. + exceptions_to_generate[error_type] = True + # Resource methods are shoved in here too but are + # identifiable: + functions_to_patch.append(function) + + fastly_compute = Path(__file__).parent.parent.parent / "fastly_compute" + write_files( + exception_code_tree(exceptions_to_generate.keys()), + fastly_compute / "exceptions", + ) + write_files( + mappings_code_tree(exceptions_to_generate.keys(), functions_to_patch), + fastly_compute, + ) diff --git a/scripts/generate_patches/utils.py b/scripts/generate_patches/utils.py new file mode 100644 index 0000000..66e9d52 --- /dev/null +++ b/scripts/generate_patches/utils.py @@ -0,0 +1,38 @@ +"""Little helpers used in patch generation""" + +import textwrap + + +def only(iterable): + """Return the one and only item of the iterable, raising ValueError if there + are more or fewer than one. + """ + items = list(iterable) + if (len_ := len(items)) != 1: + raise ValueError(f"Iterable had {len_} items, not 1.") + return items[0] + + +def upper_camel(s: str) -> str: + """Convert lower-kebab case to UpperCamelCase.""" + return "".join(word.capitalize() for word in s.split("-")) + + +def lower_snake(s: str) -> str: + """Convert lower-kebab case to lower_snake_case.""" + return s.replace("-", "_") + + +def shouty_snake(s: str) -> str: + """Convert lower-kebab case to SHOUTY_SNAKE_CASE.""" + return s.replace("-", "_").upper() + + +def indent(s: str): + """Indent as for a docstring. + + Indent all but the first line of a string by 4 spaces, strip leading and + trailing whitespace, and put a newline at the end if there's more than 1 + line. + """ + return textwrap.indent(s, " ").strip() diff --git a/scripts/generate_patches/wit.py b/scripts/generate_patches/wit.py new file mode 100644 index 0000000..a452129 --- /dev/null +++ b/scripts/generate_patches/wit.py @@ -0,0 +1,370 @@ +"""Abstraction over a WIT file + +Provides affordances for walking among WIT constructs and translating drawing +correspondences between them, componentize-py-generated Python code, and +Fastly's own slightly higher level generated code. +""" +# We override many methods and don't want to clutter the module repeating +# identical docstrings or tagging each with @override. +# ruff: noqa D102 + +import re +from collections.abc import Iterable +from types import NoneType +from typing import Any, Self + +from .utils import indent, lower_snake, only, shouty_snake, upper_camel + + +class DocsHaver: + """A WIT item which has documentation + + Abstract. + """ + + _me: Any + + def docs(self) -> str: + """Return the documentation of the type, "" if omitted.""" + return self._me.get("docs", {}).get("contents", "") + + def docstring_or_pass(self) -> str: + """Return a one-level-indented version of the docs suitable for use as a + docstring in an otherwise empty construct. + + Accordingly, emit "pass" if there is no docstring. + """ + return indent(self.docs()) or "pass" + + +class Thing(DocsHaver): + """Any kind of thing represented in WIT: type, function, etc. + + Abstract. + """ + + def __repr__(self): + return f"<{self.__class__.__name__} {self.name()}>" + + def name(self) -> str: + """Return the name of this type, in usual WIT kebab case.""" + return self._me["name"] + + def wit_module_path(self) -> str: + """Return the full dotted path to the Python module in which I am defined.""" + return "wit_world.imports." + lower_snake(self.interface().name()) + + def wit_path(self): + """Return the dotted path to my definition in wit_world. + + This is used as the key fed to ``remap_wit_errors()`` for a type, + among other things. + """ + return self.wit_module_path() + "." + upper_camel(self.name()) + + def py_exception_name(self) -> str: + """Return my name, fashioned as a suitable name for an exception.""" + return upper_camel(self.name()) + + def interface(self) -> "Interface": + """Return the interface where this type is defined.""" + raise NotImplementedError + + +class Type(Thing): + """A WIT type: primitive, stock, or user-defined. + + In practice, many types, like variants and the unit type, are represented by + more-specific subclasses, leaving this one to stand in for ones we haven't + needed to specializze for yet. + """ + + @classmethod + def from_id( + cls, type_id: int | str | None, wit_json: dict[str, list[dict]] + ) -> Self: + """Construct a type of the given index under the WIT's "types" key. + + If that type is an alias (which is the case when referencing a type from + a different interface), chase it down to its ultimate resolution. + Construct an Enum, Variant, or other more specific class if there is one. + + :arg type_id: The (non-negative) array index of the type in the WIT's type array + :arg wit_json: The entire JSON-decided WIT file + """ + while True: + if isinstance(type_id, str): + # It's a primitive. + return cls(type_id, {"name": type_id}, wit_json) + elif isinstance(type_id, NoneType): + return NullType() + + # It's an int. Chase that down, including following type aliases. + current_type = wit_json["types"][type_id] + next_type = current_type["kind"].get("type") + if next_type is None: + kind = only(current_type["kind"].keys()) + class_ = KINDS_TO_CLASSES.get(kind, cls) + return class_(type_id, current_type, wit_json) + else: + # It's a pointer to a different tyoe. + type_id = next_type + + def __init__(self, id: int | str, type_: dict, wit_json: dict[str, list[dict]]): + """Private constructor. Use from_id() instead.""" + self._id: int | str = id + self._me: dict[str, Any] = type_ + self._wit = wit_json + + def __hash__(self): + """Let us put Types into dicts and constrain them unique. + + Type instances compare and hash based on their IDs: their positions in + the WIT's type array. Only non-alias type IDs occur in instances. + """ + return hash(self._id) + + def __eq__(self, other): + return self._id == other._id + + def __ne__(self, other): + return not (self == other) + + def has_cases(self) -> bool: + return False + + def cases(self) -> Iterable["Case"]: + """Return the cases of an type if it has them, else an empty iterable.""" + return [] + + def interface(self) -> "Interface": + return Interface( + self._wit["interfaces"][self._me["owner"]["interface"]], self._wit + ) + + def py_package(self) -> str: + """Return the innermost, undotted package in which this type resides.""" + return lower_snake(self.interface().name()) + + def py_module(self) -> str: + """Return the name of the file (minus ".py") in which the exception + corresponding to this type resides. + """ + raise NotImplementedError( + "Only variants, enums, and records are currently handled as " + "``result`` error types. Looks like it's time to support others!" + ) + + def py_module_path(self) -> str: + """Return the dotted import path of the module holding the exception + corresponding to this type. + """ + raise NotImplementedError( + "Only variants, enums, and records are currently handled as " + "``result`` error types. Looks like it's time to support others!" + ) + + +class Result(Type): + """A WIT ``result``""" + + def error_type(self) -> Type: + """Return the type of my error case.""" + return self.from_id(self._me["kind"]["result"]["err"], self._wit) + + +class NullType(Type): + def __init__(self): + self._id = "null" + self._me = {"name": "null"} + + +class Record(Type): + """A WIT ``record`` type""" + + def py_module(self) -> str: + return "__init__" + + def py_module_path(self) -> str: + return f"fastly_compute.exceptions.{self.py_package()}" + + +class CaseHaver(Type): + """Abstract WIT type that has cases""" + + _case_class: type + + def has_cases(self) -> bool: + return True + + def cases(self): + return ( + self._case_class(c, self) for c in self._me["kind"][self._case_key]["cases"] + ) + + def py_module(self) -> str: + return lower_snake(self.name()) + + def py_module_path(self) -> str: + return f"fastly_compute.exceptions.{self.py_package()}.{self.py_module()}" + + +class Case(Thing): + """Abstract arm of a WIT type that has alternative manifestations.""" + + def __init__(self, case_json: dict[str, Any], haver: CaseHaver): + self._me = case_json + self._haver = haver + + +class EnumCase(Case): + """An arm of a WIT ``enum``""" + + def wit_path(self) -> str: + return self._haver.wit_path() + "." + shouty_snake(self.name()) + + +class VariantCase(Case): + """An arm of a WIT ``variant``""" + + def wit_path(self) -> str: + return ( + self._haver.wit_module_path() + + "." + + upper_camel(self._haver.name()) + + "_" + + upper_camel(self.name()) + ) + + +class Enum(CaseHaver): + _case_key = "enum" + _case_class = EnumCase + + +class Variant(CaseHaver): + _case_key = "variant" + _case_class = VariantCase + + +KINDS_TO_CLASSES = { + "enum": Enum, + "variant": Variant, + "record": Record, + "result": Result, +} + + +METHOD_RE = re.compile(r"\[(static|method|constructor)\]([a-z0-9%-]+)\.([a-z0-9%-]+)") +FREESTANDING_FUNCTION_RE = re.compile(r"[a-z0-9%-]+") + + +class Function(Thing): + """A function or resource method in a WIT""" + + def __init__( + self, + function_json: dict[str, Any], + interface_json: dict[str, Any], + wit_json: dict[str, list[dict]], + ): + self._me = function_json + self._interface = interface_json + self._wit = wit_json + + def interface(self) -> "Interface": + """Return the interface to which I belong.""" + return Interface(self._interface, self._wit) + + def wit_path(self) -> str: + """Return the dotted path to my definition in wit_world. + + This is used as the key fed to ``remap_wit_errors()`` for this type, + among other things. + """ + name = self._me["name"] + if match := METHOD_RE.match(name): + return ( + self.wit_module_path() + + "." + + upper_camel(match.group(2)) + + "." + + lower_snake(match.group(3)) + ) + elif FREESTANDING_FUNCTION_RE.match(name): + return self.wit_module_path() + "." + lower_snake(name) + else: + raise NotImplementedError( + f'A new and exciting kind of function needs to be recognized. Its name field is "{name}".' + ) + + def error_type_of_returned_result(self) -> Type | None: + """If this Function returns a single ``result`` type, return the type of its error case. + + Otherwise, return None. + """ + return_type = Type.from_id(self._me.get("result"), self._wit) + if isinstance(return_type, Result): + return return_type.error_type() + + +class Interface: + """A WIT interface""" + + def __init__(self, interface_json: dict[str, Any], wit_json: dict[str, list[dict]]): + self._me = interface_json + self._wit = wit_json + + def name(self) -> str: + return self._me["name"] + + def functions(self) -> Iterable[Function]: + """Return the functions and methods defined in this interface.""" + for function in self._me["functions"].values(): + yield Function(function, self._me, self._wit) + + +class Package: + """A WIT package""" + + def __init__(self, package_json: dict, wit_json: dict[str, list[dict]]): + self._package = package_json + self._wit = wit_json + + def interfaces(self) -> Iterable[Interface]: + """Return the iterfaces defined in this package.""" + for interface_num in self._package["interfaces"].values(): + yield Interface(self._wit["interfaces"][interface_num], self._wit) + + +class Wit: + """A WIT file + + This provides an abstraction layer atop the output of ``wasm-tools component + wit --json``. It begins a tree of classes which work their way steadily + narrower into the WIT: package, then interface, then function or type. They + are instantiated lazily, for the most part, retaining unprocessed bits of + JSON for later instantiation. + """ + + def __init__(self, wit_json: dict[str, list[dict]]): + """Construct. + + :arg wit_json: The loaded JSON out of ``wasm-tools component wit wit/ --json`` + """ + self._packages: dict[str, Package] = { + p["name"]: Package(p, wit_json) for p in wit_json["packages"] + } + + def package(self, name: str) -> Package: + """Return a package of the given name, e.g. + "fastly:compute@0.0.0-prerelease.0", or raise KeyError. + """ + return self._packages[name] + + def fastly_compute_package(self) -> Package: + """Return the package representing the Fastly Compute API.""" + package_name = only( + [p for p in self._packages.keys() if p.startswith("fastly:compute@")] + ) + return self.package(package_name) diff --git a/tests/test_nice_exceptions.py b/tests/test_nice_exceptions.py index 5b276ac..b660fb3 100644 --- a/tests/test_nice_exceptions.py +++ b/tests/test_nice_exceptions.py @@ -15,8 +15,8 @@ from fastly_compute.exceptions import ( FastlyError, UnexpectedFastlyError, - remap_wit_errors, ) +from fastly_compute.wit_patching.decorators import remap_wit_errors class BufferTooShortError(FastlyError): From 3469497709f05c74da37dd85811905aacea92ae1 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Wed, 28 Jan 2026 16:45:33 -0500 Subject: [PATCH 2/4] Get tests passing, except for ones which catch the old exceptions. Mostly, don't crash trying to monkeypatch nonexistent hostcalls when imported by the testrunner in the absence of Viceroy. Can't patch hostcalls in the absence of a host! --- fastly_compute/wit_patching/decorators.py | 2 +- scripts/generate_patches/generation.py | 50 +++++++++++++---------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/fastly_compute/wit_patching/decorators.py b/fastly_compute/wit_patching/decorators.py index 8dbcb1d..a59b2c1 100644 --- a/fastly_compute/wit_patching/decorators.py +++ b/fastly_compute/wit_patching/decorators.py @@ -5,7 +5,7 @@ from functools import wraps from typing import Any -from wit_world.imports.types import Err +from componentize_py_types import Err from fastly_compute.exceptions import FastlyError, UnexpectedFastlyError diff --git a/scripts/generate_patches/generation.py b/scripts/generate_patches/generation.py index 039f5a7..ef4263d 100755 --- a/scripts/generate_patches/generation.py +++ b/scripts/generate_patches/generation.py @@ -107,39 +107,45 @@ def mappings_code_tree( imports.add(func.wit_module_path()) # Do templating: + code += "try:\n" code += ( - "from .decorators import remap_wit_errors\n" # Linter: don't join these lines. - "import fastly_compute.exceptions\n" + " from .decorators import remap_wit_errors\n" + " import fastly_compute.exceptions\n" ) for import_ in sorted(imports): - code += f"import {import_}\n" - code += "\n\n" - - code += "MAPPINGS = {\n" + code += f" import {import_}\n" + code += ( + "except ImportError:\n" + " # Tolerate that momentary import for the testrunner before Viceroy, and thus\n" + " # the wit_world, is around.\n" + " def patch():\n" + ' print("Faking the run of exception-mapping monkeypatches for test runner.")\n' + "else:\n" + " MAPPINGS = {\n" + ) for wit_path, py_module_path, py_exception_name in sorted(mappings): - code += f" {wit_path}: {py_module_path}.{py_exception_name},\n" - code += " type(None): fastly_compute.exceptions.FastlyError,\n" - code += "}\n\n" + code += f" {wit_path}: {py_module_path}.{py_exception_name},\n" + code += ( + " type(None): fastly_compute.exceptions.FastlyError,\n" # Linter: don't wrap. + " }\n" + ) code += ''' -did_patch = False - - -def patch(): - """Apply patches if they haven't already been applied.""" + did_patch = False - global did_patch - if did_patch: - # This test shouldn't be needed, but it avoids double-wrapping the - # routines if somehow patch() did get called twice. - return - did_patch = True + def patch(): + """Apply patches if they haven't already been applied.""" -''' + global did_patch + if did_patch: + # This test shouldn't be needed, but it avoids double-wrapping the + # routines if somehow patch() did get called twice. + return + did_patch = True\n\n''' for func in functions_to_patch: func_path = func.wit_path() - code += f" {func_path} = remap_wit_errors(MAPPINGS)({func_path})\n" + code += f" {func_path} = remap_wit_errors(MAPPINGS)({func_path})\n" # TODO: Make affordance for manually adding ergonomic getter properties, # __str__s, etc. to exception classes. From 2b76077bff543e78771037b98c939b5f4da00280 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Wed, 28 Jan 2026 19:58:48 -0500 Subject: [PATCH 3/4] =?UTF-8?q?Port=20`requests`=20fa=C3=A7ade=20to=20the?= =?UTF-8?q?=20new=20exceptions.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a quick one-to-one port to get things running. Something we should consider before release is to map ErrorWithDetail to one of a set of exceptions corresponding to the detail enumeration if present, otherwise falling back to the error itself. It's not important to the `requests` package since it just turns around and does its own mapping to a `requests`-style exception hierarchy, but it'd be a big ergonomic win for normal callers. --- fastly_compute/requests/__init__.py | 19 ++++++----- fastly_compute/requests/backend.py | 21 +++++------- fastly_compute/requests/exceptions.py | 49 +++++++++++++++++++-------- fastly_compute/utils.py | 6 ++-- 4 files changed, 57 insertions(+), 38 deletions(-) diff --git a/fastly_compute/requests/__init__.py b/fastly_compute/requests/__init__.py index 01970d9..784cd8e 100644 --- a/fastly_compute/requests/__init__.py +++ b/fastly_compute/requests/__init__.py @@ -41,9 +41,10 @@ import urllib.parse from typing import Any, TypedDict, Unpack -from componentize_py_types import Err from wit_world.imports import http_body, http_req +from fastly_compute.exceptions.http_req import ErrorWithDetail +from fastly_compute.exceptions.types.error import Error from fastly_compute.requests.backend import resolve_backend from .exceptions import ( @@ -241,8 +242,8 @@ def request( wit_request = http_req.Request.new() wit_request.set_method(method.upper()) wit_request.set_uri(url_parsed.geturl()) - except Err as e: - raise RequestException.from_wit_error(e, "create_req") from e + except Error as e: + raise RequestException.from_fastly_error(e, "create_req") from e # Set headers headers = headers if headers is not None else {} @@ -270,8 +271,8 @@ def request( for name, value in headers.items(): try: wit_request.insert_header(name, value.encode("utf-8")) - except Err as e: - raise RequestException.from_wit_error(e, "insert_header") from e + except Error as e: + raise RequestException.from_fastly_error(e, "insert_header") from e # Prepare request body wit_body = http_body.new() @@ -280,17 +281,17 @@ def request( written = 0 while written < len(body): written += http_body.write(wit_body, body) - except Err as e: - raise RequestException.from_wit_error(e, "http_body.write") from e + except Error as e: + raise RequestException.from_fastly_error(e, "http_body.write") from e # Send the request try: wit_response, response_body = http_req.send( wit_request, wit_body, resolution.backend ) - except Err as e: + except ErrorWithDetail as e: # WIT-level errors during request execution - use proper error classification - raise RequestException.from_http_req_error(e, "http_req.send") from e + raise RequestException.from_detailed_error(e, "http_req.send") from e # Wrap in FastlyResponse return FastlyResponse(wit_response, response_body, url_parsed.geturl()) diff --git a/fastly_compute/requests/backend.py b/fastly_compute/requests/backend.py index ac85cc5..5567946 100644 --- a/fastly_compute/requests/backend.py +++ b/fastly_compute/requests/backend.py @@ -10,9 +10,10 @@ from dataclasses import dataclass from typing import TYPE_CHECKING -from componentize_py_types import Err from wit_world.imports import backend as wit_backend -from wit_world.imports.types import OpenError + +from fastly_compute.exceptions.types.error import Error +from fastly_compute.exceptions.types.open_error import OpenError from .exceptions import MissingSchema, RequestException @@ -65,14 +66,10 @@ def resolve_backend( # Check if backend exists by trying to open it try: backend_obj = wit_backend.Backend.open(fastly_backend) - except Err as e: - # Check if this is an OpenError (backend not found) - if isinstance(e.value, OpenError): - raise RequestException( - f"Static backend '{fastly_backend}' does not exist" - ) from e - # Re-raise if it's a different error - raise + except OpenError as e: + raise RequestException( + f"Static backend '{fastly_backend}' does not exist" + ) from e else: # dynamic backend if not parsed.scheme or not parsed.netloc: @@ -117,5 +114,5 @@ def _register_dynamic_backend( return wit_backend.register_dynamic_backend( prefix=backend_name, target=parsed_url.netloc, options=options ) - except Err as e: - raise RequestException.from_wit_error(e, "register_dynamic_backend") from e + except Error as e: + raise RequestException.from_fastly_error(e, "register_dynamic_backend") from e diff --git a/fastly_compute/requests/exceptions.py b/fastly_compute/requests/exceptions.py index c304413..e8332bb 100644 --- a/fastly_compute/requests/exceptions.py +++ b/fastly_compute/requests/exceptions.py @@ -6,8 +6,6 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from componentize_py_types import Err as WitErr - from .response import FastlyResponse # Runtime imports needed for error mappings at module level @@ -15,6 +13,17 @@ from wit_world.imports import types as wit_types from wit_world.imports.http_req import SendErrorDetail +from fastly_compute.exceptions import FastlyError +from fastly_compute.exceptions.http_req import ErrorWithDetail +from fastly_compute.exceptions.types.error import ( + CannotRead, + HttpHeadTooLarge, + HttpIncomplete, + HttpInvalid, + HttpInvalidStatus, + HttpUser, +) + def _map_error_to_exception( error: object, @@ -59,19 +68,19 @@ def __init__( self.request: http_req.Request | None = request @classmethod - def from_http_req_error( - cls, err: WitErr[http_req.ErrorWithDetail], operation: str + def from_detailed_error( + cls, err: ErrorWithDetail, operation: str ) -> RequestException: - """Create appropriate exception from http_req WIT error. + """Create a ``requests`` exception from an ErrorWithDetail. Args: - err: WIT Err exception containing ErrorWithDetail + err: The error to map from operation: Description of what operation failed Returns: Appropriate RequestException subclass instance """ - error_with_detail = err.value + error_with_detail = err.args[0] # Try detailed error classification first; this is not guaranteed # to be present in all cases. @@ -92,21 +101,19 @@ def from_http_req_error( ) @classmethod - def from_wit_error( - cls, err: WitErr[wit_types.Error], operation: str - ) -> RequestException: - """Create appropriate exception from generic WIT error. + def from_fastly_error(cls, err: FastlyError, operation: str) -> RequestException: + """Create a ``requests`` exception from a FastlyError or subclass. Args: - err: WIT Err exception containing generic Error + err: The error to map from operation: Description of what operation failed Returns: Appropriate RequestException subclass instance """ return _map_error_to_exception( - err.value, - WIT_ERROR_MAPPINGS, + err, + FASTLY_ERROR_MAPPINGS, f"Operation {operation} failed", cls, ) @@ -200,3 +207,17 @@ class StreamConsumedError(RequestException, TypeError): } ) ) + +# Map FastlyErrors to the errors `requests` returns. +FASTLY_ERROR_MAPPINGS: MappingProxyType[type[FastlyError], type[RequestException]] = ( + MappingProxyType( + { + HttpInvalid: HTTPError, + HttpUser: HTTPError, + HttpIncomplete: HTTPError, + HttpHeadTooLarge: HTTPError, + HttpInvalidStatus: HTTPError, + CannotRead: ConnectionError, + } + ) +) diff --git a/fastly_compute/utils.py b/fastly_compute/utils.py index d928c1d..ce29e92 100644 --- a/fastly_compute/utils.py +++ b/fastly_compute/utils.py @@ -1,8 +1,8 @@ """Utility functions for fastly_compute package.""" -from componentize_py_types import Err from wit_world.imports import async_io, http_body +from fastly_compute.exceptions.types.error import Error from fastly_compute.requests.exceptions import RequestException @@ -25,8 +25,8 @@ def read_response_body( while True: try: chunk = http_body.read(response_body, chunk_size) - except Err as e: - raise RequestException.from_wit_error(e, "http_body.read") from e + except Error as e: + raise RequestException.from_fastly_error(e, "http_body.read") from e if len(chunk) == 0: break From 1894e1f2b8c3cfc97568c64f14e7138ebc299cc8 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Thu, 29 Jan 2026 15:21:28 -0500 Subject: [PATCH 4/4] Rename wit_patching folder to runtime_patching. I think it's better to emphasize that we're patching at runtime. Are we patching the WIT? Not really; we're patching stuff generated from the WIT. This is less misleading. --- .gitignore | 2 +- Makefile | 10 +++++----- fastly_compute/__init__.py | 2 +- .../{wit_patching => runtime_patching}/__init__.py | 0 .../{wit_patching => runtime_patching}/decorators.py | 0 scripts/generate_patches/generation.py | 2 +- tests/test_nice_exceptions.py | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) rename fastly_compute/{wit_patching => runtime_patching}/__init__.py (100%) rename fastly_compute/{wit_patching => runtime_patching}/decorators.py (100%) diff --git a/.gitignore b/.gitignore index 026b456..f299143 100644 --- a/.gitignore +++ b/.gitignore @@ -7,7 +7,7 @@ __pycache__ /stubs/ /fastly_compute/exceptions/* !/fastly_compute/exceptions/__init__.py -/fastly_compute/wit_patching/patches.py +/fastly_compute/runtime_patching/patches.py # Build artifacts /build/ diff --git a/Makefile b/Makefile index 6b9f08a..4815115 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,7 @@ $(STUBS_DIR): $(COMPUTE_WIT) uv run componentize-py -d wit --world-module wit_world -w $(TARGET_WORLD) bindings $(STUBS_DIR) # Build our composed wasm using fastly-compute-py build -$(BUILD_DIR)/%.composed.wasm: wit/viceroy.wit wit/deps/fastly/compute.wit fastly_compute/wsgi.py fastly_compute/wit_patching/patches.py | $(BUILD_DIR) $(STUBS_DIR) +$(BUILD_DIR)/%.composed.wasm: wit/viceroy.wit wit/deps/fastly/compute.wit fastly_compute/wsgi.py fastly_compute/runtime_patching/patches.py | $(BUILD_DIR) $(STUBS_DIR) @echo "Building $* example with fastly-compute-py..." @test -d $(EXAMPLES_DIR)/$* || (echo "Error: Example directory $(EXAMPLES_DIR)/$* not found" && exit 1) @test -f $(EXAMPLES_DIR)/$*/$*.py || (echo "Error: Example file $(EXAMPLES_DIR)/$*/$*.py not found" && exit 1) @@ -60,7 +60,7 @@ $(BUILD_DIR)/%.composed.wasm: wit/viceroy.wit wit/deps/fastly/compute.wit fastly # The script that writes the exceptions and the patches always rewrites # everything, so we can depend on the mod date of only 1 file. We choose # patches.py, because its name doesn't depend on the WIT contents. -fastly_compute/wit_patching/patches.py: scripts/generate_patches/*.py $(COMPUTE_WIT) +fastly_compute/runtime_patching/patches.py: scripts/generate_patches/*.py $(COMPUTE_WIT) uv run python -m scripts.generate_patches # Create build directory @@ -88,12 +88,12 @@ list-examples: # Clean build artifacts clean: rm -rf $(BUILD_DIR) $(STUBS_DIR) - rm -f fastly_compute/wit_patching/patches.py + rm -f fastly_compute/runtime_patching/patches.py cd fastly_compute/exceptions && rm -rf acl http_body http_req kv_store types cd crates/fastly-compute-py && cargo clean # Development tools -lint: fastly_compute/wit_patching/patches.py | $(STUBS_DIR) +lint: fastly_compute/runtime_patching/patches.py | $(STUBS_DIR) @echo "Checking version synchronization..." uv run python scripts/check_version_sync.py @echo "Linting Python code..." @@ -102,7 +102,7 @@ lint: fastly_compute/wit_patching/patches.py | $(STUBS_DIR) @echo "Linting Rust code..." cd crates/fastly-compute-py && cargo clippy -- -D warnings -lint-fix: fastly_compute/wit_patching/patches.py +lint-fix: fastly_compute/runtime_patching/patches.py @echo "Fixing Python code..." uv run --extra dev ruff check --fix . @echo "Fixing Rust code..." diff --git a/fastly_compute/__init__.py b/fastly_compute/__init__.py index 5d7d2e4..35a6eed 100644 --- a/fastly_compute/__init__.py +++ b/fastly_compute/__init__.py @@ -6,7 +6,7 @@ # Testing utilities are available but not imported by default # Users can import them explicitly: from fastly_compute.testing import ViceroyTestBase -from fastly_compute.wit_patching.patches import patch +from fastly_compute.runtime_patching.patches import patch # Before anything from the fastly_compute package is used, do our monkeypatching # to make the WIT-generated code act more Pythonically: diff --git a/fastly_compute/wit_patching/__init__.py b/fastly_compute/runtime_patching/__init__.py similarity index 100% rename from fastly_compute/wit_patching/__init__.py rename to fastly_compute/runtime_patching/__init__.py diff --git a/fastly_compute/wit_patching/decorators.py b/fastly_compute/runtime_patching/decorators.py similarity index 100% rename from fastly_compute/wit_patching/decorators.py rename to fastly_compute/runtime_patching/decorators.py diff --git a/scripts/generate_patches/generation.py b/scripts/generate_patches/generation.py index ef4263d..4d8f742 100755 --- a/scripts/generate_patches/generation.py +++ b/scripts/generate_patches/generation.py @@ -153,7 +153,7 @@ def patch(): # TODO: Maybe automatically improve the docstring of each method to list the # exceptions it raises. - return {"wit_patching": {"patches.py": code}} + return {"runtime_patching": {"patches.py": code}} def write_files(tree: Mapping[str, Mapping[str, str]], base_folder: Path): diff --git a/tests/test_nice_exceptions.py b/tests/test_nice_exceptions.py index b660fb3..b3f20d2 100644 --- a/tests/test_nice_exceptions.py +++ b/tests/test_nice_exceptions.py @@ -16,7 +16,7 @@ FastlyError, UnexpectedFastlyError, ) -from fastly_compute.wit_patching.decorators import remap_wit_errors +from fastly_compute.runtime_patching.decorators import remap_wit_errors class BufferTooShortError(FastlyError):