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

Missing attributes on services fails with poor quality debug message #1085

Open
msglm opened this issue Feb 19, 2025 · 2 comments
Open

Missing attributes on services fails with poor quality debug message #1085

msglm opened this issue Feb 19, 2025 · 2 comments
Milestone

Comments

@msglm
Copy link

msglm commented Feb 19, 2025

This code in bugwarrior/collect.py will yield the issue, which is to my knowledge always an Object, when there is a missing Attribute. To my knowledge, every instance where aggregate_issues (or some passed-down version of it) is iterated over needs a dictionary returned. Yielding an object creates a type error that is difficult to debug.

        try:
            yield TaskConstructor(issue).get_taskwarrior_record()
        except AttributeError:
            if isinstance(issue, tuple):
                currently_running -= 1
                completion_type, target = issue
                if completion_type == SERVICE_FINISHED_ERROR:
                    log.error(f"Aborted [{target}] due to critical error.")
                    yield ('SERVICE FAILED', target)
                continue
            yield issue

Please change the line "yield issue" to "raise".

@ryneeverett
Copy link
Collaborator

I agree that yielding the issue no longer makes sense here because the subsequent code expects either an issue dictionary or a tuple of information. However, I don't think we want to raise the error here because one badly behaving service shouldn't cause the others to fail too. I'm thinking that instead we should simply log the error rather than yield it -- so change yield issue to log.exception(f'AttributeError on {issue.config.target}'). What do you think?

@ryneeverett ryneeverett added this to the release-next milestone Feb 20, 2025
@ryneeverett
Copy link
Collaborator

After sleeping on this I'm thinking your original solution was correct. Any other kind of exception in the service would raise an uncaught exception here and we only happen to be catching AttributeError because that is how we detect our informational tuples. If it's not an informational tuple we should treat it like any other exception.

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

No branches or pull requests

2 participants