Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: Improve runtime type checking error message #4333

Merged
merged 1 commit into from Jan 9, 2023

Conversation

helderco
Copy link
Contributor

@helderco helderco commented Jan 9, 2023

Fixes #4212

Overview

Error messages come from the beartype project. They provide much greater detail on typos or wrong value types when using the client.

Codegen was refactored a bit to accommodate:

  • Scalars are now classes instead of NewType which was only useful in static type checking.
  • list was changed to Sequence in field inputs1 to allow, for example tuples or other types of sequences, instead of only a list.
  • The internal Arg was reverted to be simpler because we no longer need to pass context for the error message there.
  • A @typecheck decorator was added to all methods that we want to do runtime type checking. It has all context for the origin of the error.
  • Changed how forward references are created to help beartype:
    • Optional["DirectoryID"] instead of "DirectoryID | None" (notice quotes only around undefined type instead of the whole annotation).
    • Now there's context on which types have been defined or not, to only "escape" the ones that aren't.

Traceback improvements

  • Much shorter distance from the offending line to the raised exception; 🙌
  • Much more detail on which parameter type and value doesn't match what's expected; 🎉

Before

...
  File "/Users/helder/Projects/dagger/sdk/python/examples/client-simple/say.py", line 23, in main
    .with_directory("/a", await workdir.id())
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helder/Projects/dagger/sdk/python/src/dagger/api/gen.py", line 437, in with_directory
    _ctx = self._select("withDirectory", _args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helder/Projects/dagger/sdk/python/src/dagger/api/base.py", line 167, in _select
    self._convert_args(args),
    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helder/Projects/dagger/sdk/python/src/dagger/api/base.py", line 181, in _convert_args
    raise TypeError(
TypeError: Wrong type for 'directory' parameter. Expected a 'Directory' instead.

After

...
  File "/Users/helder/Projects/dagger/sdk/python/examples/client-simple/say.py", line 23, in main
    .with_directory("/a", await workdir.id())
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helder/Projects/dagger/sdk/python/src/dagger/api/base.py", line 234, in wrapper
    raise err from None
TypeError: dagger.Container.with_directory() parameter directory='eyJsbGIiOnsiZGVmIjpbIkdyY0JDakJzYjJOaGJEb3ZMeTlWYzJWeWN5OW9aV3hrWlhJdlVISnZhbVZqZEhNdlpHRm5a... violates type hint 'Directory', as <class "dagger.DirectoryID"> 'eyJsbGIiOnsiZGVmIjpbIkdyY0JDakJzYjJOaGJEb3ZMeTlWYzJWeWN5OW9aV3hrWlhJdlVISnZhbVZqZEhNdlpHRm5a... not instance of <protocol "dagger.Directory">.

Note: Values are actually colored to stand out.

Screenshot 2023-01-09 at 10 48 33

Coroutines

We now detect if you forgot to add an await. Notice at the end: Did you forget to await?

...
  File "/Users/helder/Projects/dagger/sdk/python/examples/client-simple/say.py", line 20, in main
    dir = client.directory(workdir.id())
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helder/Projects/dagger/sdk/python/src/dagger/api/base.py", line 234, in wrapper
    raise err from None
TypeError: dagger.Client.directory() parameter id=<coroutine object Directory.id at 0x105cb1b40> violates type hint typing.Optional[dagger.DirectoryID], as <class "builtins.coroutine"> <coroutine object Directory.id at 0x105cb1b40> not <class "builtins.NoneType"> or <class "dagger.DirectoryID">. Did you forget to await?

Screenshot 2023-01-09 at 10 54 18

This tells you to:

- dir = client.directory(workdir.id())
+ dir = client.directory(await workdir.id())

Signed-off-by: Helder Correia

Footnotes

  1. It's good practice to have broad inputs and narrow output in regards to type checking.

Copy link
Member

@grouville grouville left a comment

Choose a reason for hiding this comment

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

LGTM. Once the failing test cases gets changed, you can shipit.

Tested locally, on a small repro:

Type error

Given this code snippet:

# Collect value of SSH_AUTH_SOCK env var, to retrieve authentication socket path
ssh_auth_sock_path = await client.host().env_variable("SSH_AUTH_SOCK").secret()
            
# Retrieve authentication socket from host
ssh_agent_socket = client.host().unix_socket(ssh_auth_sock_path)

We get this output:

python3 toto.py
object Secret can't be used in 'await' expression

Await failure

Given this code snippet:

# Collect value of SSH_AUTH_SOCK env var, to retrieve authentication socket path
ssh_auth_sock_path = client.host().env_variable("SSH_AUTH_SOCK").value()
            
# Retrieve authentication socket from host
ssh_agent_socket = client.host().unix_socket(ssh_auth_sock_path)

Here, I purposely forget to await on ssh_auth_sock_path = client.host().env_variable("SSH_AUTH_SOCK").value() line.

We get this feedback:
image

The error message has a structure to which we need to adapt, but once you get the hang of it, it really helps debugging where we should have had put the await.

👏

Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
@grouville grouville merged commit c541313 into dagger:main Jan 9, 2023
@helderco helderco deleted the python/runtime-typecheck branch January 9, 2023 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python: Improve error message in runtime type checking
2 participants