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

Failures in struct packing should fail and not hang forever #1319

Open
jeffwidman opened this issue Dec 8, 2017 · 2 comments
Open

Failures in struct packing should fail and not hang forever #1319

jeffwidman opened this issue Dec 8, 2017 · 2 comments

Comments

@jeffwidman
Copy link
Collaborator

jeffwidman commented Dec 8, 2017

If we try to build a struct using incorrect types, then _pack() is correctly raising an error. However, the foreground thread just hangs when this happens.

This doesn't make sense to me--it's a client side error, so we should be able to fail loudly to the user. The error may be slightly cryptic because all the struct knows is that something is incorrect, it has no idea of the broader context. But at least it'd give the user something to go on...

This behavior may also apply to _unpack().

To reproduce, in _pack(), manually insert a conditional that flips a type:

def _pack(f, value):
    try:
        if isinstance(value, int):
            value = str(value)
        return pack(f, value)
    except error:
        raise ValueError(error)

See also #1318

@jeffwidman jeffwidman changed the title Failures in struct packing should not hang forever Failures in struct packing should fail loudly and not hang forever Dec 8, 2017
@jeffwidman jeffwidman changed the title Failures in struct packing should fail loudly and not hang forever Failures in struct packing should fail and not hang forever Dec 8, 2017
@jeffwidman jeffwidman reopened this Dec 8, 2017
@tvoinarovskyi
Copy link
Collaborator

@jeffwidman Just to confirm, we talk about the producer here, yes?

@jeffwidman
Copy link
Collaborator Author

I only tested on the producer. As best I can tell, this hanging is a result of the interaction between threads. So the current version of the consumer, being single-threaded, will not have this problem. However, the addition of the background thread in #1266 may introduce this for the consumer as well... I haven't tested that.

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