Skip to content
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

shutdown genserver without raising exception #269

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

mruoss
Copy link
Collaborator

@mruoss mruoss commented Aug 11, 2023

When an adapter needs to be terminated, we should pass a reason that doesn't treat the termination like a crash of the process and therefore doesn't raise an exception.

refs #268


Requirements for all pull requests

  • Entry in CHANGELOG.md was created

@mruoss mruoss linked an issue Aug 11, 2023 that may be closed by this pull request
If the connection is closed, we don't want to wait another 30s for the process to shut down. Otherwise there might be no viable adapter in the pool for 30s.
@mruoss mruoss force-pushed the stop-genserver-without-raising branch from e8ec55b to 48030c6 Compare August 11, 2023 20:37
Copy link
Contributor

@elliottneilclark elliottneilclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks good.


@spec shutdown_if_closed(t()) :: {:noreply, t()} | {:stop, {:shutdown, :closed}, t()}
defp shutdown_if_closed(state) do
if Mint.HTTP.open?(state.conn) or any_non_empty_buffers?(state) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this condition is repeated above. Would it be worth putting this into a function? Then you can also explain why the server is not shutdown even if mint has closed the connection, but there are buffers

@mruoss mruoss merged commit 18e9281 into develop Aug 15, 2023
12 of 13 checks passed
@mruoss mruoss deleted the stop-genserver-without-raising branch December 25, 2023 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mint HTTPAdapter failure cycle spams errors
2 participants