Skip to content

Do not report a registered type as "Unknown type" in get_cuda_native_handle - #2551

Open
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:native-handle-keyerror-scope
Open

Do not report a registered type as "Unknown type" in get_cuda_native_handle#2551
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:native-handle-keyerror-scope

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

get_cuda_native_handle() wraps both the registry lookup and the getter call in a
single try:

    obj_type = type(obj)
    try:
        return _handle_getters[obj_type](obj)
    except KeyError:
        raise TypeError("Unknown type: " + str(obj_type)) from None

The except KeyError is there for one thing: "this type has no registered getter". But
_handle_getters[obj_type](obj) is two operations, so a KeyError raised inside the
getter is caught too — and then the diagnosis is wrong twice over:

  • the reported type is registered, so "Unknown type" sends the reader looking in
    exactly the wrong place;
  • from None sets __suppress_context__, so the traceback that would have shown the real
    failure is not printed at all.

Reproduced (no GPU needed):

>>> _add_cuda_native_handle_getter(Registered, getter_that_raises_keyerror)
>>> get_cuda_native_handle(Registered())
TypeError: Unknown type: <class 'Registered'>

Fix

Move the getter call out of the try, so only the dict lookup is guarded. The
unregistered-type path is byte-for-byte unchanged, and the existing
test_get_handle_error still covers it.

This is a latent mis-diagnosis rather than a crash — today's registered getters are
generated one-liners — but it is the kind of "error path that fires for the wrong reason
and then destroys the evidence" that #2122 is about, and the fix is three lines with no
behaviour change on either existing path.

Tests

One test added next to the existing test_get_handle / test_get_handle_error pair in
cuda_bindings/tests/test_utils.py: it registers a getter that raises KeyError (via
monkeypatch.setitem, so the global registry is restored) and asserts the KeyError
propagates. Fails on main with TypeError: Unknown type: <class 'Registered'>.

Verified against upstream/main and with the change; the happy path and the
unregistered-type path both keep passing either way. ruff check and
ruff format --check clean.

Note: #2541 also touches cuda_bindings/tests/test_utils.py, appending at the end of the
file; this change lands mid-file next to the existing handle tests, so the two are
independent. Happy to rebase whichever lands second.

…handle

get_cuda_native_handle() wraps both the registry lookup and the getter call
in one try:

    try:
        return _handle_getters[obj_type](obj)
    except KeyError:
        raise TypeError("Unknown type: " + str(obj_type)) from None

The except clause is meant for "this type has no registered getter", but it
also fires for a KeyError raised *inside* the getter. When that happens the
diagnosis is wrong twice over: the reported type is registered, and
`from None` suppresses the context so the traceback that would show the real
failure is gone.

    >>> _add_cuda_native_handle_getter(Registered, getter_that_raises_keyerror)
    >>> get_cuda_native_handle(Registered())
    TypeError: Unknown type: <class 'Registered'>

Move the getter call out of the try. The unregistered-type path is
unchanged, which the existing test_get_handle_error still covers.
@copy-pr-bot

copy-pr-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.bindings Everything related to the cuda.bindings module label Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant