Skip to content

warnings, getpass, os, adafruit_bus_device, traceback: failures repor… - #11383

Open
peterbay wants to merge 1 commit into
adafruit:mainfrom
peterbay:failures-reported-as-success
Open

peterbay wants to merge 1 commit into
adafruit:mainfrom
peterbay:failures-reported-as-success

Conversation

@peterbay

Copy link
Copy Markdown

Code written by Claude Code, guided and corrected by @peterbay.

The problem

Five places where a failure is reported as a success, or a caller's state is left changed by a call that did not succeed. One of them faults the board.

The changes

  • warnings.warn built the exception past the category. mp_raise_msg_str asserts that the type's make_new is mp_obj_exception_make_new. A Python subclass of Warning has mp_obj_instance_make_new instead, and the assert is compiled out of a release build, so the wrong function was called with the exception's arguments. It calls the category now and raises what comes back.

  • getpass used its prompt as a format string. mp_printf(print, prompt) reads conversions out of a caller-supplied string, so getpass("Enter 100% of the key: ") swallowed the % and the character after it and pulled a word off the varargs area for it. Changed to mp_print_str.

  • os.chdir committed the new directory before the call that can fail. MP_STATE_VM(cwd_path) and vfs_cur were written first, then the lookup and the proxy call, with no restore on the raise path — so a chdir to a missing directory raised and left the current directory pointing at it. They are written after the call succeeds.

  • adafruit_bus_device returned as though it held the bus when an exception was pending. The acquire loop broke out on mp_hal_is_interrupted() and the function then carried on; the first transfer inside the with block ran without the lock. It calls mp_handle_pending(true), which raises what is waiting.

  • traceback.print_exception did not restore what it had overwritten if printing raised. It deliberately replaces the live exception's traceback, and its __context__ and __cause__ when chain is false, and puts them back afterwards — but only on the path where printing returned. The restore is on both paths now.

Testing

Seeed XIAO nRF52840 Sense, on two builds differing only by these changes.

before after
os.chdir("/no/such/directory") raises, then os.getcwd() /no/such/directory /, unchanged
os.chdir("/lib") on a directory that exists /lib, unchanged either way /lib
getpass("Enter 100% of the key: ", stream), prompt as written Enter 100of the key: Enter 100% of the key:
warn("hello", Warning) under simplefilter("error") raises Warning: hello, unchanged either way raises Warning: hello
warn("hello", MyWarning), a Python subclass hard fault raises MyWarning: hello

The two warn rows together are the point: the built-in category works on both builds, because its make_new is the one the assert expects. It is only a category defined in Python that reaches the wrong function.

Two of the changes are not in that table. Reaching the traceback restore needs printing itself to raise, and nothing I could arrange from Python got it to — the file argument is checked for writability before anything is overwritten, and a custom __str__ that raises is not called on the path that prints. The adafruit_bus_device lock needs a bus and an exception pending at the moment the lock is taken.

No new translatable strings.

…ted as success

mp_raise_msg_str asserts that the type's make_new is mp_obj_exception_make_new.
A Python subclass of Warning has mp_obj_instance_make_new instead, and the
assert is compiled out of a release build, so warnings.warn called the wrong
function with the exception's arguments and faulted the board. It calls the
category and raises what comes back.

getpass passed its prompt to mp_printf as the format string, so a prompt
containing a percent sign read conversions off the varargs area.

os.chdir wrote cwd_path and vfs_cur before the lookup and the proxy call that
can fail, with no restore on the raise path, so a chdir to a missing directory
raised and left the current directory pointing at it.

adafruit_bus_device broke out of its acquire loop when an exception was
pending and then returned as though it held the bus; it calls
mp_handle_pending(true) now.

traceback.print_exception overwrites the live exception's traceback, and its
context and cause when chain is false, and restored them only on the path
where printing returned.
@peterbay

Copy link
Copy Markdown
Author

Testing and diagnostic script.
failures_reported_as_success.py

@tannewt tannewt left a comment

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.

Please have your LLM add tests into tests/ that would have failed without these fixes. If the unix port doesn't have these modules then use the zephyr native_sim tests.

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.

2 participants