GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-131878: Handle top level exceptions in new pyrepl and prevent of closing it by sergey-miryanov · Pull Request #131910 · python/cpython

Conversation

@sergey-miryanov

@sergey-miryanov sergey-miryanov commented Mar 30, 2025

Copy link
Copy Markdown
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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sergey-miryanov
Handle top level exceptions in new pyrepl and prevent of closing it
56f0784
@sergey-miryanov
sergey-miryanov requested review from ambv, lysnikolaou and pablogsal as code owners March 30, 2025 14:56
@bedevere-app bedevere-app Bot added the awaiting review label Mar 30, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Mar 30, 2025
New REPL on Windows exits when accented character is pasted/typed #131878
Closed
@sergey-miryanov
Add news entry
e380a24
@picnixz

picnixz commented Mar 30, 2025

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sergey-miryanov

sergey-miryanov commented Mar 30, 2025

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Mar 30, 2025

Copy link
Copy Markdown
Member

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?)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sergey-miryanov

sergey-miryanov commented Mar 30, 2025

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@python-cla-bot

python-cla-bot Bot commented Apr 6, 2025

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ambv
ambv reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_pyrepl/simple_interact.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@ambv
Don't swallow process exit codes
ccc0806
@ambv ambv added the needs backport to 3.13 only security fixes label May 5, 2025
@ambv
ambv merged commit 99a0d7e into python:main May 5, 2025
@miss-islington-app

miss-islington-app Bot commented May 5, 2025

Copy link
Copy Markdown

Thanks @sergey-miryanov for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting review label May 5, 2025
@miss-islington-app

miss-islington-app Bot commented May 5, 2025

Copy link
Copy Markdown

Sorry, @sergey-miryanov and @ambv, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 99a0d7e5b3c6e80f5b9b0ec88ae439d6d620253c 3.13
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app miss-islington-app Bot assigned ambv May 5, 2025
ambv added a commit to ambv/cpython that referenced this pull request May 5, 2025
@sergey-miryanov @ambv
[3.13] pythongh-131878: Handle top level exceptions in new pyrepl and…
b75310d
… prevent of closing it (pythonGH-131910)

(cherry picked from commit 99a0d7e)

Co-authored-by: Sergey Miryanov <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
@bedevere-app

bedevere-app Bot commented May 5, 2025

Copy link
Copy Markdown

GH-133445 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label May 5, 2025
ambv added a commit that referenced this pull request May 5, 2025
@ambv @sergey-miryanov
[3.13] gh-131878: Handle top level exceptions in new pyrepl and preve…
87b14c8
…nt of closing it (GH-131910) (GH-133445)

Co-authored-by: Sergey Miryanov <[email protected]>
@sergey-miryanov

sergey-miryanov commented May 5, 2025

Copy link
Copy Markdown
Contributor Author

@ambv Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sergey-miryanov
sergey-miryanov deleted the gh-131878-handle-top-level-exceptions-in-the-new-repl branch May 5, 2025 15:55
@sergey-miryanov sergey-miryanov mentioned this pull request May 11, 2025
New REPL exits when there are non-string candidates for suggestions #130999
Closed
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@sergey-miryanov @ambv @Pranjal095
pythongh-131878: Handle top level exceptions in new pyrepl and preven…
dce813c
…t of closing it (python#131910)

Co-authored-by: Łukasz Langa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ambv ambv ambv left review comments
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@ambv ambv

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sergey-miryanov @picnixz @ambv
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.