Skip to content

gh-131878: Handle top level exceptions in new pyrepl and prevent of closing it #131910

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sergey-miryanov
Copy link
Contributor

Working on gh-131878 I saw a code paths that swallows exceptions. I propose not to swallow them and bubble it up to main input loop and handle there.

@picnixz
Copy link
Member

picnixz commented Mar 30, 2025

AFAICT, the swallowed exceptions are already suppressed when executing the statements. When would this except be hit? it's only if we're raising in one of the following:

            ps1 = getattr(sys, "ps1", ">>> ")
            ps2 = getattr(sys, "ps2", "... ")
            try:
                statement = multiline_input(more_lines, ps1, ps2)
            except EOFError:
                break

            if maybe_run_command(statement):
                continue

            input_name = f"<python-input-{input_n}>"
            more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single")  # type: ignore[call-arg]
            assert not more
            input_n += 1

And in this case, I think it's preferrable to make the REPL exit as it's probably in an unrecoverable state at this point.

@sergey-miryanov
Copy link
Contributor Author

I came from this https://github.com/python/cpython/blob/main/Lib/_pyrepl/base_eventqueue.py#L108-L114.

We swallow UnicodeError exception. And this is a reason why gh-131878 asserted in another point. And if we caught UnicodeError here it give us much more information. But here we don't have any "tools" to properly say about exception. And going up to stack I "found" an appropriate place in the simple_interact loop.

OTOH I may be wrong, and we should not do this.

@picnixz
Copy link
Member

picnixz commented Mar 30, 2025

I think the issue is that we did not properly handled the inputs there. Or that the assertion where it crashes is incorrect. I didn't look in detail but I don't know if #131901 is the correct approach (ISTM that we cannot send long unicode strings but we can do it as bytes? and that we can do it for unicode strings with a single unicode code point, whether it's strings or bytes?)

Nevertheless, when there is a crash like that, I think it's better than we use a debugger instead. At this point of the REPL execution, its state may or may not necessarily be recoverable (I'm actually surprised that we're not aborting because we're out of memory but I guess it's to allow debuggers?)

@sergey-miryanov
Copy link
Contributor Author

ISTM that we cannot send long unicode strings but we can do it as bytes? and that we can do it for unicode strings with a single unicode code point, whether it's strings or bytes?

As long it affects only Windows so I changed only windows code path. And for both cases (if input has one or more unicode code path) input "char" encoded to bytes and sent to eventqueue. For one-byte unicode string it is a small pessimization.

For original issue - we have input == "ñ". If we send it as-is - it sends as str and ord("ñ") == 241. And we just append it to buf. And, if we append "ñ" to buf and try to decode it - it will fail, because it is incomplete unicode char.

But if we encode it as bytes then we get two bytes - b'\xc3\xb1', also append them (via extend) to buf and decode will be happy.

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 6, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

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