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

test: test coverage for throwing Bandit plug call #17

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

grzuy
Copy link
Collaborator

@grzuy grzuy commented Oct 22, 2024

No description provided.

@grzuy grzuy changed the title fix: reports bandit throw fix: reports throw from inside Bandit plug call Oct 22, 2024
# An exit instead of a throw because Bandit doesn't handle throw's
# for the moment. See: https://github.com/mtrudel/bandit/pull/396.
"class" => "(exit)",
"message" => "bad return value: \"from inside a plug\"",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After digging a bit deeper on why a throw within Bandit ends up being a "bad return value" exit, realized that it's because, given Bandit doesn't handle throws, and that the plug call runs inside a :gen_server, the thrown value gets interpreted as the :gen_server callback return value. See mtrudel/bandit#410.

So given that we're testing with throwing a string here, and that is an invalid :gen_server return value, we do get termination of the gen_server process and an exit event.

However, if the thrown value were to coincide with a valid :gen_server return value, no event would be raised. Extremely unlikely I guess, but technically possible.

@@ -28,7 +28,7 @@ defmodule TowerHoneybadger.Honeybadger.Notice do
stacktrace: stacktrace,
plug_conn: plug_conn
}) do
error_notice("(exit)", reason, stacktrace, plug_conn)
error_notice("(exit)", Exception.format_exit(reason), stacktrace, plug_conn)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moving this to a more generalized PR #18.
This shouldn't be a Bandit specific change.

@grzuy grzuy changed the title fix: reports throw from inside Bandit plug call test: reports throw from inside Bandit plug call Oct 24, 2024
@grzuy grzuy changed the title test: reports throw from inside Bandit plug call test: test coverage for throwing Bandit plug call Oct 24, 2024
@grzuy grzuy merged commit 84b3049 into main Oct 24, 2024
6 checks passed
@grzuy grzuy deleted the bandit-throw branch October 24, 2024 19:13
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.

1 participant