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

Fix for Issue 166 #196

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

digitalsleuth
Copy link

The error identified in #166 is caused when attempting to log e.message, however e does not actually have an attribute message. This PR removes the .message from the call to e, resolving the issue. Solution here is originally credited to the author of the issue.

Copy link

google-cla bot commented Nov 22, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@digitalsleuth
Copy link
Author

@tinajohnson As discussed, any chance you can review this?

@tinajohnson
Copy link
Contributor

Hey @digitalsleuth,
Thanks for the PR. We would want to fix all occurrences of this error-prone code in the codebase. Would you be able to work on them? If not, I am happy to take over.

@digitalsleuth
Copy link
Author

Hi @tinajohnson , I wouldn't be opposed to helping out, however I don't have as much of a familiarity with the entirety of the code as you or someone else might have. If it's simply a matter of removing .message from each instance of e, that can be done, but testing each function to confirm the attribute is not needed would take some time.

Do you have any tests which execute each function already available to that I might be able to use to test with?

Aside from that, I do have another PR in the chamber ready to submit for another error, but I wanted to wait until I could get fakenet running again with this PR before I submit any others.

@digitalsleuth
Copy link
Author

Hi @tinajohnson , I've reviewed the similar calls to e.message throughout and replaced them with str(e) instead to provide the proper error output.

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.

2 participants