Skip to content

Feature: Add an option to generate exceptions for Conjure errors#1306

Open
wsuppiger wants to merge 4 commits into
developfrom
wsuppiger/errors-fix
Open

Feature: Add an option to generate exceptions for Conjure errors#1306
wsuppiger wants to merge 4 commits into
developfrom
wsuppiger/errors-fix

Conversation

@wsuppiger

@wsuppiger wsuppiger commented Jun 3, 2026

Copy link
Copy Markdown

branch off: #1157

For issue: #910

After this PR

A few things happening:

  1. Threading through a CLI option for generating the errors.
  2. Core generation logic.
  3. Used Claude Code to generate the tests for me.

The resulting error look like:

class product_DatasetNotFound(ConjureHTTPError):
    """Thrown when the requested dataset does not exist
    """

    ERROR_CODE = "NOT_FOUND"
    ERROR_NAMESPACE = "Datasets"
    ERROR_NAME = "DatasetNotFound"

    class SafeArgs(TypedDict):
        dataset_rid: str
        available_datasets: List[str]

    def __init__(self, base_error: ConjureHTTPError) -> None:
        super().__init__(
            status_code=base_error.status_code,
            error_code=base_error.error_code,
            error_name=base_error.error_name,
            error_instance_id=base_error.error_instance_id,
            parameters=base_error.parameters
        )
        self.safe_args: product_DatasetNotFound.SafeArgs = {
            'dataset_rid': base_error.parameters['datasetRid'],
            'available_datasets': base_error.parameters['availableDatasets']
        }

    @classmethod
    def is_instance(cls, error: ConjureHTTPError) -> bool:
        return (
            error.error_name == cls.ERROR_NAME and
            error.error_code == cls.ERROR_CODE
        )

    @classmethod
    def from_error(cls, error: ConjureHTTPError) -> 'product_DatasetNotFound':
        if not cls.is_instance(error):
            raise ValueError(f"Error is not a {cls.ERROR_NAME}")
        return cls(error)


product_DatasetNotFound.__name__ = "DatasetNotFound"
product_DatasetNotFound.__qualname__ = "DatasetNotFound"
product_DatasetNotFound.__module__ = "package_name.product"

Also placed a follow-up item for using the enum for ErrorCode after palantir/conjure-python-client#171.

@changelog-app

changelog-app Bot commented Jun 3, 2026

Copy link
Copy Markdown

Successfully generated changelog entry!

Entry generated via PR title

To modify this entry, edit PR title using proper format.


📋Changelog Preview

✨ Features

  • Add an option to generate exceptions for Conjure errors (#1306)

namespace: Datasets
code: INVALID_ARGUMENT
safe-args:
fileSystemId: string

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor: Include docs for one of the args here

.build(),
PythonImport.of("builtins"),
PythonImport.builder()
.moduleSpecifier(ImportTypeVisitor.TYPING)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This import will be unused if both safeArgs and unsafeArgs are empty.

poetWriter.writeIndentedLine("return (");
poetWriter.increaseIndent();
poetWriter.writeIndentedLine("error.error_name == cls.ERROR_NAME and");
poetWriter.writeIndentedLine("error.error_code == cls.ERROR_CODE");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor: I'd only check error name, for consistency with conjure-java and conjure-typescript.

import org.immutables.value.Value;

@Value.Immutable
public interface ErrorSnippet extends PythonSnippet {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The generated error class doesn't provide a way to generate an instance from just the args (unlike conjure-java, which does). I'm assuming this is because you didn't intend for this to be used server-side, only to extract errors to a more convenient type on the client side? Is it worth generating constructors as well, and decoupling this from needing to extend ConjureHTTPError?

default void emitConstructor(PythonPoetWriter poetWriter) {
poetWriter.writeIndentedLine("def __init__(self, base_error: ConjureHTTPError) -> None:");
poetWriter.increaseIndent();
poetWriter.writeIndentedLine("super().__init__(");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

poetWriter.writeIndentedLine("super().__init__(");
poetWriter.increaseIndent();
poetWriter.writeIndentedLine("status_code=base_error.status_code,");
// TODO(bzhang): Use enum once https://github.com/palantir/conjure-python-client/pull/171 is merged

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor: this comment shouldn't appear in the final PR; we should either merge and pick up here, or remove the comment (if we don't want to merge)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants