This error is the top-level error for the client. Every error surfaced by - * the client MUST be a subclass of this so that customers can reliably catch all - * exceptions it raises. The client implementation will wrap any errors that aren't - * already subclasses. + *
This error is the top-level error for modeled client errors. * * @param settings The client settings, used to account for module configuration. * @return Returns the symbol for the client's error class. @@ -105,40 +102,6 @@ public static Symbol getServiceError(PythonSettings settings) { .build(); } - /** - * Gets the service API error symbol. - * - *
This error is the parent class for all errors returned over the wire by the - * service, including unknown errors. - * - * @param settings The client settings, used to account for module configuration. - * @return Returns the symbol for the client's API error class. - */ - public static Symbol getApiError(PythonSettings settings) { - return Symbol.builder() - .name("ApiError") - .namespace(String.format("%s.models", settings.moduleName()), ".") - .definitionFile(String.format("./src/%s/models.py", settings.moduleName())) - .build(); - } - - /** - * Gets the unknown API error symbol. - * - *
This error is the parent class for all errors returned over the wire by - * the service which aren't in the model. - * - * @param settings The client settings, used to account for module configuration. - * @return Returns the symbol for unknown API errors. - */ - public static Symbol getUnknownApiError(PythonSettings settings) { - return Symbol.builder() - .name("UnknownApiError") - .namespace(String.format("%s.models", settings.moduleName()), ".") - .definitionFile(String.format("./src/%s/models.py", settings.moduleName())) - .build(); - } - /** * Gets the symbol for the http auth params. * diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java index 4c7907983..a44f7cb91 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java @@ -9,7 +9,6 @@ import java.util.Locale; import java.util.logging.Logger; import software.amazon.smithy.codegen.core.ReservedWordSymbolProvider; -import software.amazon.smithy.codegen.core.ReservedWords; import software.amazon.smithy.codegen.core.ReservedWordsBuilder; import software.amazon.smithy.codegen.core.Symbol; import software.amazon.smithy.codegen.core.SymbolProvider; @@ -84,6 +83,10 @@ public PythonSymbolProvider(Model model, PythonSettings settings) { var reservedMemberNamesBuilder = new ReservedWordsBuilder() .loadWords(PythonSymbolProvider.class.getResource("reserved-member-names.txt"), this::escapeWord); + // Reserved words that only apply to error members. + var reservedErrorMembers = new ReservedWordsBuilder() + .loadWords(PythonSymbolProvider.class.getResource("reserved-error-member-names.txt"), this::escapeWord); + escaper = ReservedWordSymbolProvider.builder() .nameReservedWords(reservedClassNames) .memberReservedWords(reservedMemberNamesBuilder.build()) @@ -92,13 +95,8 @@ public PythonSymbolProvider(Model model, PythonSettings settings) { .escapePredicate((shape, symbol) -> !StringUtils.isEmpty(symbol.getDefinitionFile())) .buildEscaper(); - // Reserved words that only apply to error members. - ReservedWords reservedErrorMembers = reservedMemberNamesBuilder - .put("code", "code_") - .build(); - errorMemberEscaper = ReservedWordSymbolProvider.builder() - .memberReservedWords(reservedErrorMembers) + .memberReservedWords(reservedErrorMembers.build()) .escapePredicate((shape, symbol) -> !StringUtils.isEmpty(symbol.getDefinitionFile())) .buildEscaper(); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ServiceErrorGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ServiceErrorGenerator.java index 6bc84a761..8cc09d1d3 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ServiceErrorGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ServiceErrorGenerator.java @@ -4,7 +4,6 @@ */ package software.amazon.smithy.python.codegen.generators; -import java.util.Set; import software.amazon.smithy.codegen.core.WriterDelegator; import software.amazon.smithy.python.codegen.CodegenUtils; import software.amazon.smithy.python.codegen.PythonSettings; @@ -30,38 +29,15 @@ public void run() { var serviceError = CodegenUtils.getServiceError(settings); writers.useFileWriter(serviceError.getDefinitionFile(), serviceError.getNamespace(), writer -> { writer.addDependency(SmithyPythonDependency.SMITHY_CORE); - writer.addImport("smithy_core.exceptions", "SmithyException"); + writer.addImport("smithy_core.exceptions", "ModeledException"); writer.write(""" - class $L(SmithyException): - ""\"Base error for all errors in the service.""\" - pass - """, serviceError.getName()); - }); - - var apiError = CodegenUtils.getApiError(settings); - writers.useFileWriter(apiError.getDefinitionFile(), apiError.getNamespace(), writer -> { - writer.addStdlibImports("typing", Set.of("Literal", "ClassVar")); - var unknownApiError = CodegenUtils.getUnknownApiError(settings); - - writer.write(""" - @dataclass - class $1L($2T): - ""\"Base error for all API errors in the service.""\" - code: ClassVar[str] - fault: ClassVar[Literal["client", "server"]] + class $L(ModeledException): + ""\"Base error for all errors in the service. - message: str - - def __post_init__(self) -> None: - super().__init__(self.message) - - - @dataclass - class $3L($1L): - ""\"Error representing any unknown api errors.""\" - code: ClassVar[str] = 'Unknown' - fault: ClassVar[Literal["client", "server"]] = "client" - """, apiError.getName(), serviceError, unknownApiError.getName()); + Some exceptions do not extend from this class, including + synthetic, implicit, and shared exception types. + ""\" + """, serviceError.getName()); }); } } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/SetupGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/SetupGenerator.java index fedc5b612..021ba4019 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/SetupGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/SetupGenerator.java @@ -451,7 +451,6 @@ private static void writeIndexes(GenerationContext context, String projectName) writeIndexFile(context, "docs/models/index.rst", "Models"); } - /** * Write the readme in the docs folder describing instructions for generation * @@ -461,18 +460,18 @@ private static void writeDocsReadme( GenerationContext context ) { context.writerDelegator().useFileWriter("docs/README.md", writer -> { - writer.write(""" - ## Generating Documentation - - Sphinx is used for documentation. You can generate HTML locally with the - following: - - ``` - $$ uv pip install ".[docs]" - $$ cd docs - $$ make html - ``` - """); + writer.write(""" + ## Generating Documentation + + Sphinx is used for documentation. You can generate HTML locally with the + following: + + ``` + $$ uv pip install ".[docs]" + $$ cd docs + $$ make html + ``` + """); }); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java index e0e3d2dcd..410dcb0cc 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java @@ -31,6 +31,7 @@ import software.amazon.smithy.model.traits.InputTrait; import software.amazon.smithy.model.traits.OutputTrait; import software.amazon.smithy.model.traits.RequiredTrait; +import software.amazon.smithy.model.traits.RetryableTrait; import software.amazon.smithy.model.traits.SensitiveTrait; import software.amazon.smithy.model.traits.StreamingTrait; import software.amazon.smithy.python.codegen.CodegenUtils; @@ -130,31 +131,40 @@ private void renderError() { writer.addStdlibImports("typing", Set.of("Literal", "ClassVar")); writer.addStdlibImport("dataclasses", "dataclass"); - // TODO: Implement protocol-level customization of the error code var fault = errorTrait.getValue(); - var code = shape.getId().getName(); var symbol = symbolProvider.toSymbol(shape); - var apiError = CodegenUtils.getApiError(settings); + var baseError = CodegenUtils.getServiceError(settings); writer.pushState(new ErrorSection(symbol)); + writer.putContext("retryable", false); + writer.putContext("throttling", false); + + var retryableTrait = shape.getTrait(RetryableTrait.class); + if (retryableTrait.isPresent()) { + writer.putContext("retryable", true); + writer.putContext("throttling", retryableTrait.get().getThrottling()); + } writer.write(""" @dataclass(kw_only=True) class $1L($2T): - ${5C|} + ${4C|} - code: ClassVar[str] = $3S - fault: ClassVar[Literal["client", "server"]] = $4S + fault: Literal["client", "server"] | None = $3S + ${?retryable} + is_retry_safe: bool | None = True + ${?throttling} + is_throttle: bool = True + ${/throttling} + ${/retryable} + + ${5C|} - message: str ${6C|} ${7C|} - ${8C|} - """, symbol.getName(), - apiError, - code, + baseError, fault, writer.consumer(w -> writeClassDocs(true)), writer.consumer(w -> writeProperties()), @@ -325,7 +335,9 @@ private void writeMemberDocs(MemberShape member) { String memberName = symbolProvider.toMemberName(member); String docs = writer.formatDocs(String.format(":param %s: %s%s", - memberName, descriptionPrefix, trait.getValue())); + memberName, + descriptionPrefix, + trait.getValue())); writer.write(docs); }); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/HttpProtocolGeneratorUtils.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/HttpProtocolGeneratorUtils.java index b8841d26f..c8de9b77d 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/HttpProtocolGeneratorUtils.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/HttpProtocolGeneratorUtils.java @@ -136,26 +136,30 @@ public static void generateErrorDispatcher( var transportResponse = context.applicationProtocol().responseType(); var delegator = context.writerDelegator(); var errorDispatcher = context.protocolGenerator().getErrorDeserializationFunction(context, operation); - var apiError = CodegenUtils.getApiError(context.settings()); - var unknownApiError = CodegenUtils.getUnknownApiError(context.settings()); var canReadResponseBody = canReadResponseBody(operation, context.model()); delegator.useFileWriter(errorDispatcher.getDefinitionFile(), errorDispatcher.getNamespace(), writer -> { writer.pushState(new ErrorDispatcherSection(operation, errorShapeToCode, errorMessageCodeGenerator)); + writer.addImport("smithy_core.exceptions", "CallException"); + // TODO: include standard retry-after in the pure-python version of this writer.write(""" - async def $1L(http_response: $2T, config: $3T) -> $4T: - ${6C|} + async def $1L(http_response: $2T, config: $3T) -> CallException: + ${4C|} match code.lower(): - ${7C|} + ${5C|} case _: - return $5T(f"{code}: {message}") + is_throttle = http_response.status == 429 + return CallException( + message=f"{code}: {message}", + fault="client" if http_response.status < 500 else "server", + is_throttle=is_throttle, + is_retry_safe=is_throttle or None, + ) """, errorDispatcher.getName(), transportResponse, configSymbol, - apiError, - unknownApiError, writer.consumer(w -> errorMessageCodeGenerator.accept(context, w, canReadResponseBody)), writer.consumer(w -> errorCases(context, w, operation, errorShapeToCode))); writer.popState(); diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/MarkdownToRstDocConverter.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/MarkdownToRstDocConverter.java index 52415c8d8..b2ab2173d 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/MarkdownToRstDocConverter.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/MarkdownToRstDocConverter.java @@ -77,7 +77,7 @@ public void head(Node node, int depth) { String text = textNode.text(); if (!text.trim().isEmpty()) { if (text.startsWith(":param ")) { - int secondColonIndex = text.indexOf(':', 1); + int secondColonIndex = text.indexOf(':', 1); writer.write(text.substring(0, secondColonIndex + 1)); //TODO right now the code generator gives us a mixture of // RST and HTML (for instance :param xyz:
docs @@ -85,7 +85,7 @@ public void head(Node node, int depth) { // starts a newline. We account for that with this if/else // statement, but we should refactor this in the future to // have a more elegant codepath. - if (secondColonIndex +1 == text.strip().length()) { + if (secondColonIndex + 1 == text.strip().length()) { writer.indent(); writer.ensureNewline(); } else { @@ -97,8 +97,8 @@ public void head(Node node, int depth) { } else { writer.writeInline(text); } - // Account for services making a paragraph tag that's empty except - // for a newline + // Account for services making a paragraph tag that's empty except + // for a newline } else if (node.parent() instanceof Element && ((Element) node.parent()).tagName().equals("p")) { writer.writeInline(text.replaceAll("[ \\t]+", "")); } diff --git a/codegen/core/src/main/resources/software/amazon/smithy/python/codegen/reserved-error-member-names.txt b/codegen/core/src/main/resources/software/amazon/smithy/python/codegen/reserved-error-member-names.txt new file mode 100644 index 000000000..e1b05dff9 --- /dev/null +++ b/codegen/core/src/main/resources/software/amazon/smithy/python/codegen/reserved-error-member-names.txt @@ -0,0 +1,4 @@ +is_retry_safe +retry_after +is_throttle +fault diff --git a/designs/exceptions.md b/designs/exceptions.md new file mode 100644 index 000000000..97acbb3ff --- /dev/null +++ b/designs/exceptions.md @@ -0,0 +1,124 @@ +# Exceptions + +Exceptions are a necessary aspect of any software product (Go notwithstanding), +and care must be taken in how they're exposed. This document describes how +smithy-python clients will expose exceptions to customers. + +## Goals + +* Every exception raised by a Smithy client should be catchable with a single, + specific catch statement (that is, not just `except Exception`). +* Every modeled exception raised by a service should be catchable with a single, + specific catch statement. +* Exceptions should contain information about retryablility where relevant. + +## Specification + +Every exception raised by a Smithy client MUST inherit from `SmithyException`. + +```python +class SmithyException(Exception): + """Base exception type for all exceptions raised by smithy-python.""" +``` + +If an exception that is not a `SmithyException` is thrown while executing a +request, that exception MUST be wrapped in a `SmithyException` and the +`__cause__` MUST be set to the original exception. + +Just as in normal Python programming, different exception types SHOULD be made +for different kinds of exceptions. `SerializationException`, for example, will +serve as the exception type for any exceptions that occur while serializing a +request. + +### Retryability + +Not all exceptions need to include information about retryability, as most will +not be retryable at all. To avoid overly complicating the class hierarchy, +retryability properties will be standardized as a `Protocol` that exceptions MAY +implement. + +```python +@dataclass(kw_only=True) +@runtime_checkable +class RetryInfo(Protocol): + is_retry_safe: bool | None = None + """Whether the exception is safe to retry. + + A value of True does not mean a retry will occur, but rather that a retry is allowed + to occur. + + A value of None indicates that there is not enough information available to + determine if a retry is safe. + """ + + retry_after: float | None = None + """The amount of time that should pass before a retry. + + Retry strategies MAY choose to wait longer. + """ + + is_throttle: bool = False + """Whether the error is a throttling error.""" +``` + +If an exception with `RetryInfo` is received while attempting to send a +serialized request to the server, the contained information will be used to +inform the next retry. + +### Service Exceptions + +Exceptions returned by the service MUST be a `CallException`. `CallException`s +include a `fault` property that indicates whether the client or server is +responsible for the exception. HTTP protocols can determine this based on the +status code. + +Similarly, protocols can and should determine retry information. HTTP protocols +can generally be confident that a status code 429 is a throttling error and can +also make use of the `Retry-After` header. Specific protocols may also include +more information in protocol-specific headers. + +```python +type Fault = Literal["client", "server"] | None +"""Whether the client or server is at fault. + +If None, then there was not enough information to determine fault. +""" + + +@dataclass(kw_only=True) +class CallException(SmithyException, RetryInfo): + fault: Fault = None + message: str = field(default="", kw_only=False) +``` + +#### Modeled Exceptions + +Most exceptions thrown by a service will be present in the Smithy model for the +service. These exceptions will all be generated into the client package. Each +modeled exception will be inherit from a generated exception named +`ServiceException` which itself inherits from the static `ModeledException`. + +```python +@dataclass(kw_only=True) +class ModeledException(CallException): + """Base exception to be used for modeled errors.""" +``` + +The Smithy model itself can contain fault information in the +[error trait](https://smithy.io/2.0/spec/type-refinement-traits.html#smithy-api-error-trait) +and retry information in the +[retryable trait](https://smithy.io/2.0/spec/behavior-traits.html#retryable-trait). +This information will be statically generated onto the exception. + +```python +@dataclass(kw_only=True) +class ServiceException(ModeledException): + pass + + +@dataclass(kw_only=True) +class ThrottlingException(ServcieException): + fault: Fault = "client" + is_retry_safe: bool | None = True + is_throttle: bool = True +``` diff --git a/packages/smithy-core/src/smithy_core/exceptions.py b/packages/smithy-core/src/smithy_core/exceptions.py index 0c07e30d3..63467b715 100644 --- a/packages/smithy-core/src/smithy_core/exceptions.py +++ b/packages/smithy-core/src/smithy_core/exceptions.py @@ -1,9 +1,67 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 +from dataclasses import dataclass, field +from typing import Literal, Protocol, runtime_checkable + + class SmithyException(Exception): """Base exception type for all exceptions raised by smithy-python.""" +type Fault = Literal["client", "server"] | None +"""Whether the client or server is at fault. + +If None, then there was not enough information to determine fault. +""" + + +@dataclass(kw_only=True) +@runtime_checkable +class RetryInfo(Protocol): + is_retry_safe: bool | None = None + """Whether the exception is safe to retry. + + A value of True does not mean a retry will occur, but rather that a retry is allowed + to occur. + + A value of None indicates that there is not enough information available to + determine if a retry is safe. + """ + + retry_after: float | None = None + """The amount of time that should pass before a retry. + + Retry strategies MAY choose to wait longer. + """ + + is_throttle: bool = False + """Whether the error is a throttling error.""" + + +@dataclass(kw_only=True) +class CallException(SmithyException, RetryInfo): + """Base exception to be used in application-level errors.""" + + fault: Fault = None + """Whether the client or server is at fault. + + If None, then there was not enough information to determine fault. + """ + + message: str = field(default="", kw_only=False) + """The message of the error.""" + + def __post_init__(self): + super().__init__(self.message) + + +@dataclass(kw_only=True) +class ModeledException(CallException): + """Base exception to be used for modeled errors.""" + + fault: Fault = "client" + + class SerializationException(Exception): """Base exception type for exceptions raised during serialization."""