Skip to content
Merged
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
7 changes: 5 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
# Python/uv artifacts
.venv/
*.egg-info
__pycache__

# Generated code
/stubs/
__pycache__
/fastly_compute/exceptions/*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would probably be OK and it may make for easier reference to include this generated code in tree given that the inputs probably don't change too frequently and we probably do want to examine changes that occur fairly closely.

When to do regen then becomes a bit different, potentially; perhaps do it manually with some kind of CI check to catch it containing non-cosmetic differences. Not a hill I'd die on, but I do think this could be a case where including the generated code might be worthwhile to aid reference.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

100% agree. I was wishing for that as I developed it, so I'll open a follow-up PR with that change. It's a fairly elegant way of putting this code under test.

!/fastly_compute/exceptions/__init__.py
/fastly_compute/runtime_patching/patches.py

# Build artifacts
/build/
Expand All @@ -12,4 +16,3 @@ bin/
# Rust
target/
*.so
target/
14 changes: 11 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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/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)
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/runtime_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)
Expand All @@ -82,10 +88,12 @@ list-examples:
# Clean build artifacts
clean:
rm -rf $(BUILD_DIR) $(STUBS_DIR)
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: | $(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..."
Expand All @@ -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/runtime_patching/patches.py
@echo "Fixing Python code..."
uv run --extra dev ruff check --fix .
@echo "Fixing Rust code..."
Expand Down
6 changes: 6 additions & 0 deletions fastly_compute/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.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:
patch()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm still uneasy about doing this kind of runtime patching. It introduces magic that is not necessary without any benefit other than reducing the amount of code that needs to be generated slightly.

The alternative I still believe is preferred is to have a generated layer that does the translation; this is similar to what we are patching in at runtime but generated a compile time. Use would look like this:

# Current, use wit_world import that gets magically patched
from wit_world.imports import compute_runtime

# Using generated wrapper (details flexible)
from fastly_compute.wit import compute_runtime

With that in mind, I also think we can move forward with the change as-is and it will not preclude us from changing our approach on this later on. Both would perform a similar transform, sharing most of the same code.

I'll keep thinking on this, but wanted to speak my peace that this feels like a bit of a hack that could cause us some pain and it isn't necessary to achieve what is being achieved here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Noted! I'm not ordinarily a monkeypatching kind of guy either, but I think in this case the good outweighs the bad. My motivating good is that it effectively clears the undesirable behavior out of the system; no one is going to call a low-level Err-raising routine by accident. That wouldn't ordinarily be worth much worry, except that many of our routines are methods on resources (on classes, in the Python world). Any source-level wrapping would have to carry around a duplicate of each class to house nice-exception-raising methods. Now you've got 2 different Request classes kicking around, throwing different exceptions, returning different kinds of other classes. One slip by a customer or a lib they use (should we be so successful), and you could end up with fairly subtle bugs involving same-named classes, ones which might not be discovered except under (less-tested) error conditions.

A halfway approach might be to throw some kind of warning if wit_world is imported directly. Or stow it under its_a_terrible_idea_to_import_this.wit_world etc.

It might not be a bad idea to convert this into a "2-way door" by importing everything in wit_world through to fastly_compute.wit after all (resurrecting #32). I'm 3/5 in favor of this. What do you think?

29 changes: 29 additions & 0 deletions fastly_compute/exceptions/__init__.py
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 10 additions & 9 deletions fastly_compute/requests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {}
Expand Down Expand Up @@ -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()
Expand All @@ -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())
21 changes: 9 additions & 12 deletions fastly_compute/requests/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
49 changes: 35 additions & 14 deletions fastly_compute/requests/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,24 @@
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
from wit_world.imports import http_req
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,
Expand Down Expand Up @@ -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.
Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
}
)
)
1 change: 1 addition & 0 deletions fastly_compute/runtime_patching/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Monkeypatches (and supporting machinery) which make WIT behavior more Pythonic"""
Original file line number Diff line number Diff line change
@@ -1,37 +1,15 @@
"""Top-level exceptions emitted by the Fastly API"""
"""Decorators used in runtime patching"""

from collections.abc import Callable, Mapping
from enum import Enum
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

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:
Expand Down
6 changes: 3 additions & 3 deletions fastly_compute/utils.py
Original file line number Diff line number Diff line change
@@ -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


Expand All @@ -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
Expand Down
17 changes: 5 additions & 12 deletions fastly_compute/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading