don't try to respond with bad gateway after OK closes #2977

[myleshorton]

Can you also take a look at this one @uaalto?

[uaalto]

This change is correct. I think this change fixes the situation, which currently has the case where a badGateway is responded after the OK. That is definitely a bug. We shouldn't respond BadGateway after the OK.
Alternatively, we can also check for a failed OK and reply with badGateway if that's the case.

[myleshorton]

Alternatively, we can also check for a failed OK and reply with badGateway if that's the case.

I think the only time the OK would fail in practice is if the client is no longer there though, which would of course mean the bad gateway would fail too. Does that make sense?

[uaalto]

I agree that it is very unlikely that the OK would fail. Maybe with some setup like lossy local WiFi where another device uses Lantern as a proxy. This is the sort of situation we talked recently for mobile phones in local networks.
But in any case, what I think could happen is that we fail in connOut, err = detour.Dialer(d)("tcp", addr), and it happens before the OK. At the moment we are not retrying, so we could answer badGateway in that situation and cancel the connection.

On the other hand, to simplify things, I think it would be ok to respond OK and close the connection. This would leave the browser/user-agent to deal with it.

[uaalto]

So, if you agree with last point, I merge =)

[myleshorton]

Yeah I agree it's theoretically possible, but I don't think it's quite worth going to the trouble. We could create another sync.Once method that responds and maybe preferences one response code over the other with a flag, but in practice I think the OK would only fail if the connection were actually lost and that in any case it would almost always beat the bad gateway call. So think the fact that we close the connection even after the OK should be enough for the browser.

[uaalto]

I agree. LGTM, merging!

[myleshorton]

Thanks @uaalto!