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

fixes Bug 893585 - remove thirty frame signature restriction #1337

Merged
merged 1 commit into from Jul 18, 2013
Merged

fixes Bug 893585 - remove thirty frame signature restriction #1337

merged 1 commit into from Jul 18, 2013

Conversation

twobraids
Copy link
Contributor

Way back in 2009, we restricted signatures to come from the top thirty frames of the crashing thread. Apparently, this is less useful in these modern days. This PR removes the 30 frame restriction.

if make_modules_lower_case:
try:
module_name = module_name.lower()
except AttributeError:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm generally not a fan of ignoring exceptions. Why are we not doing anything here to catch/log this exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only change to that handler in this PR is the indentation. It is out of the scope of this PR to worry about the philosophy of exception handling on code that has been in use for literally years.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and the exception is ignored because it just means that the value of module_name is None, a perfectly legal situation. there's no need to log or otherwise deal with the exception.

@brandonsavage
Copy link
Contributor

Answer works for me. r+

brandonsavage added a commit that referenced this pull request Jul 18, 2013
fixes Bug 893585 - remove thirty frame signature restriction
@brandonsavage brandonsavage merged commit 8e81557 into mozilla-services:master Jul 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants