Override rich.console pipe handler for rich 13.8.0+

[mgorny]

Explicitly override rich.console.Console.on_broken_pipe() to reraise the original exception, to bring the behavior of rich 13.8.0+ in line with older versions. The new versions instead close output fds and exit with error instead, which prevents pip's pipe handler from firing. This is the minimal change needed to make pip's test suite pass after upgrading vendored rich.

Bug #13006
Bug #13072

(I'm assuming this doesn't need a news item, since vendored rich update itself will have one)

[notatallshaw]

There were a few pushes on this PR and I'm pretty sure the comment got out of sync with what is happening:

        # Reraise the original exception, rich 13.8.0+ exits by default
        # instead, preventing our handler from firing.
        raise BrokenPipeError() from None

It looks to me that it's not reraising the original exception but instead raising a new BrokenPipeError exception to simulate the same functionality as existed before.

[notatallshaw]

@uranusjr @sbidoul Given this got merged into main is the plan to include this in a possible 24.3.2 with a possible packaging 24.1.1?

[mgorny]

There were a few pushes on this PR and I'm pretty sure the comment got out of sync with what is happening:

        # Reraise the original exception, rich 13.8.0+ exits by default
        # instead, preventing our handler from firing.
        raise BrokenPipeError() from None

It looks to me that it's not reraising the original exception but instead raising a new BrokenPipeError exception to simulate the same functionality as existed before.

Sorry about that. Ruff was unhappy with the bare raise, so I went for the next thing that triggered the same behavior.

[notatallshaw]

Sorry about that. Ruff was unhappy with the bare raise, so I went for the next thing that triggered the same behavior.

Hmm, IMO this is one of those cases where general best practise linting is suggesting the wrong thing, a callback of an exception handler where you want to preserve the exception is exactly the time a bare raise makes sense. I would have put a noqa note explaining why. But it's a fairly minor issue.

[mgorny]

I think the problem is that we'd be relying on rich not clobbering the exception, so it's a bit unsafe. That said, we're basically catching the exception right away and replacing it, so shouldn't matter much.

[sbidoul]

Given this got merged into main is the plan to include this in a possible 24.3.2 with a possible packaging 24.1.1?

That is not my plan. I don't want to take any risk with 24.3.2.

[ichard26]

Thank you @mgorny! I'll update the vendoring PR sometime tomorrow.

[mgorny]

Thanks!