Skip to content
This repository was archived by the owner on Apr 10, 2022. It is now read-only.

Questions about the ExceptionGroup API #6

Closed
1st1 opened this issue Oct 27, 2020 · 9 comments
Closed

Questions about the ExceptionGroup API #6

1st1 opened this issue Oct 27, 2020 · 9 comments

Comments

@1st1
Copy link
Member

1st1 commented Oct 27, 2020

  1. Should ExceptionGroup.__init__ have a message argument? Should it be the first argument or keyword-only?

  2. Should we allow creating an empty ExceptionGroup and allow adding exceptions to it?

  3. Should we allow raising an empty ExceptionGroup?

  4. Should ExceptionGroup.__init__ accept *errors: ExceptionGroup or errors: Iterable[ExceptionGroup]?

@1st1
Copy link
Member Author

1st1 commented Oct 27, 2020

re 1: There's a common pattern to extract the error message: error.args[0]. So perhaps it the signature of __init__ should be ExceptionGroup.__init__(message: str, /, *errors: BaseException).

re 2: I'd say keep it simple; users can simply build a list of errors and pass it to the constructor. Disallowing empty ExceptionGroups seems like more work for no particularly good reason.

re 3: I think we should. The only way to fix this is to somehow validate the ExceptionGroup just before raising it (we could do that, theoretically, in the interpreter). But that would mean than instead of getting an ExceptionGroup exception out of raise ExceptionGroup(), the user would see TypeError('ExceptionGroup requires at least one exception') which would be super confusing.

re 4: IMO *errors: ExceptionGroup is a nicer API.

@iritkatriel
Copy link
Member

re 2: I think we need an API for adding exceptions into an existing exception group anyway, for when we have to merge new exceptions into the unhandled exceptions at the end of a try.

re 3: I think so too. The only other option I can think of is that "raise ExceptionGroup()" is a NOOP. But that's also weird and probably surprising (you expected to terminate execution here, otherwise the raise would be conditional).

@1st1
Copy link
Member Author

1st1 commented Oct 27, 2020

re 2: I think we need an API for adding exceptions into an existing exception group anyway, for when we have to merge new exceptions into the unhandled exceptions at the end of a try.

Right.

re 3: I think so too. The only other option I can think of is that "raise ExceptionGroup()" is a NOOP. But that's also weird and probably surprising (you expected to terminate execution here, otherwise the raise would be conditional).

Yeah, if I'd see that raise ExceptionGroup() is a nop in my code I'd probably think I found a bug in Python. So I agree this would be indeed weird & surprising.

@gvanrossum
Copy link
Member

4\. Should `ExceptionGroup.__init__` accept `*errors: ExceptionGroup` or `errors: Iterable[ExceptionGroup]`?

Hm... I think users won't be constructing these manually much, and I think internally we'd just end up calling ExceptionGroup(*exceptions) a lot, which just causes an extra copy of a list/tuple into (relatively) precious argument stack space.

@gvanrossum
Copy link
Member

re 2: I'd say keep it simple; users can simply build a list of errors and pass it to the constructor. Disallowing empty ExceptionGroups seems like more work for no particularly good reason.

I had to read this several times and I'm still not sure what you're saying. I think you're saying that an ExceptionGroup, once created, cannot be expanded (or shrunk, I suppose), but that we'll allow users to write ExceptionGroup([]) or ExceptionGroup() (depending on how (4) rolls).

@gvanrossum
Copy link
Member

Honestly I think we should try to write up the full API for ExceptionGroup -- I expect there will be various surprises.

@1st1
Copy link
Member Author

1st1 commented Oct 27, 2020

re 2: I'd say keep it simple; users can simply build a list of errors and pass it to the constructor. Disallowing empty ExceptionGroups seems like more work for no particularly good reason.

I had to read this several times and I'm still not sure what you're saying. I think you're saying that an ExceptionGroup, once created, cannot be expanded (or shrunk, I suppose), but that we'll allow users to write ExceptionGroup([]) or ExceptionGroup() (depending on how (4) rolls).

Yes, that's exactly what I meant. I admit the quoted paragraph isn't clearly written, sigh.

Honestly I think we should try to write up the full API for ExceptionGroup -- I expect there will be various surprises.

I agree, it's time to define it. It might also affect the proposed try..except* design, and will most likely affect the TracebackGroup spec. FWIW I created this issue mostly to not forget about some points raised in the PR #5.

@iritkatriel
Copy link
Member

iritkatriel commented Jan 3, 2021

Our experimental implementation does this:

  • ExceptionGroup.__init__(message: str, /, *errors: BaseException)
  • ExceptionGroups can be empty but are immutable so cannot grow. They can be raised when empty.

I don't think the interpreter will ever need to create empty exception groups (projec/split return Py_None rather than creating an empty group).
We need to decide what we do when a user creates an empty group. I think it should be allowed, and:

  • it should match except* ExceptionGroup or except ExceptionGroup
  • but should match nothing else
  • except* T (when not issubclass(T, ExceptionGroup)) just swallows it - nothing matches and there is nothing left to raise.

@iritkatriel
Copy link
Member

I'm closing this because we have established the API by now - most of this discussion is very out of date.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants