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

error_status_hint = 400 even when we are the client #41

Open
vfaronov opened this issue Mar 25, 2017 · 2 comments
Open

error_status_hint = 400 even when we are the client #41

vfaronov opened this issue Mar 25, 2017 · 2 comments

Comments

@vfaronov
Copy link

import h11
conn = h11.Connection(our_role=h11.CLIENT)
conn.send(h11.Request(method='GET', target='/', headers=[('Host', 'foo')]))
conn.send(h11.EndOfMessage())
conn.receive_data(b'')
try:
    conn.next_event()
except h11.RemoteProtocolError as exc:
    print(exc.error_status_hint)

This prints:

400

The error_status_hint might make sense when our_role=h11.CLIENT insofar as a server may be acting as a gateway. In that case, it may be more useful to have this hint set to 502 (Bad Gateway). Or perhaps removed altogether, so that (presumably) the server’s default 5xx kicks in — but this would be a breaking API change.

@njsmith
Copy link
Member

njsmith commented Mar 25, 2017

Breaking changes are possible -- see #17. (It sounds like you might want to subscribe to #17 in any case.)

I'm​ a little unclear on how you're imagining this being used; can you elaborate? I guess the way I was conceptualizing it was, we give as precise a code as we can to describe what the error was, and then it's up to you what to do with this information. (If you're a client, probably not much.)

LocalProtocolErrors also have the hint, which is even more useless. I guess it means something like "if I had allowed you to send this (and that actually were possible), and the peer was a server at a point in the state machine where they send back an error status, then this is the one they would send"?

Which reminds me that there's also the case where we are in server mode, and get a nice error_status_hint but then can't do anything with it because we're not in a state where we can legally send a response. Maybe this is fine, but worth mentioning I guess.

We can't really change how LocalProtocolErrors work because at the point where they're generated we don't necessarily have access to connection state information (e.g. they can be raised by event object constructors). For RemoteProtocolErrors there is a single point where they're constructed and it does have access to connection state information. But I don't have a clear concept of what the goal would be?

@vfaronov
Copy link
Author

I'm​ a little unclear on how you're imagining this being used; can you elaborate?

In the (toy) app that I’m building with h11, I have a catch-all error handler modeled after your Curio example. And then inside the actual request handler I sometimes operate a gateway: I open a socket to an upstream server and forward the request to it using h11. Now, if the upstream server misbehaves, I get a RemoteProtocolError, but my gateway code doesn’t do any error handling, so it bubbles up to the catch-all, which ends up sending 400 where really 502 or 500 is called for.

Of course, I can easily catch RemoteProtocolError in the gateway code and raise some other exception in its place. So, probably a wacky idea.

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