Skip to content

Don't trap on invalid connection state transitions #1573

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

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Mar 3, 2023

Motivation:

The connection manager is quite aggresive about trapping if an invalid state is hit. It's alsmost impossible to know when states are truly unreachable so in most coses we should handle them as best as we can. If we believe them to be unreachable but cannot easily prove it then we should crash in debug mode and handle it as best as possible in release mode.

Modifications:

  • Handle various state transitions more gently in the connection manager.

Result:

Gentler state handling.

Motivation:

The connection manager is quite aggresive about trapping if an invalid
state is hit. It's alsmost impossible to know when states are truly
unreachable so in most coses we should handle them as best as we can. If
we believe them to be unreachable but cannot easily prove it then we
should crash in debug mode and handle it as best as possible in release
mode.

Modifications:

- Handle various state transitions more gently in the connection
  manager.

Result:

Gentler state handling.
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Mar 3, 2023
@glbrntt glbrntt merged commit 770629b into grpc:main Mar 3, 2023
@glbrntt glbrntt deleted the gb-trap-less branch March 3, 2023 13:27
WendellXY pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
Motivation:

The connection manager is quite aggresive about trapping if an invalid
state is hit. It's alsmost impossible to know when states are truly
unreachable so in most coses we should handle them as best as we can. If
we believe them to be unreachable but cannot easily prove it then we
should crash in debug mode and handle it as best as possible in release
mode.

Modifications:

- Handle various state transitions more gently in the connection
  manager.

Result:

Gentler state handling.
pinlin168 pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
Motivation:

The connection manager is quite aggresive about trapping if an invalid
state is hit. It's alsmost impossible to know when states are truly
unreachable so in most coses we should handle them as best as we can. If
we believe them to be unreachable but cannot easily prove it then we
should crash in debug mode and handle it as best as possible in release
mode.

Modifications:

- Handle various state transitions more gently in the connection
  manager.

Result:

Gentler state handling.
@bzeeman
Copy link

bzeeman commented Nov 15, 2023

Can I just say that this is not as gentle for anyone using this as it is for people writing this? Assertions left in code where developers that aren't familiar with its inner workings are less helpful as discardable errors or even a throwing function.

@Lukasa
Copy link
Collaborator

Lukasa commented Nov 16, 2023

These are intended to be loud because hitting them reveals critical bugs in either your code or ours. We believe hitting these transitions should be impossible, so if they aren't then we want to know about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants