Add additional logging to subscribe route and simplify calling client_connected

[gefjon]

Description of Changes

Out-of-band discussions with the BitCraft team brought up questions about whether it was possible for a rejected client connection to start an expensive computation like a subscription before their connection was killed, e.g. by sending a Subscribe message along the WebSocket before client_connected had finished returning Err.

I don't believe this was actually possible, as ClientConnection::spawn called and awaited call_identity_connected before invoking its actor closure, and it was that actor which processed Subscribe messages. But it was somewhat difficult to verify that behavior, and so I re-organized the code so that the outer layer of the subscribe handler obviously had that property without having to step into ClientConnection::spawn.

I also added some additional logging to the subscribe route, including the X-Forwarded-For header in more messages, as the BitCraft team complained about having difficulty correlating IP addresses with connections. The log levels remain the same as before, just with additional information added:

• Successful connections are at debug level,
• Rejected connections are at info level (these are the ones BitCraft cares about in this case).
• Failed connections are at warn.

As the levels are unchanged, this should not add undesirable log noise.

API and ABI breaking changes

N/a

Expected complexity level and risk

3? The subscribe route was complex and remains so. I believe this change simplifies the code and makes the logic more obvious, but reviewers should take care to verify that the behavior actually is equivalent as I believe.

Testing

• I would like a test deployment of BitCraft to staging or something, or to include this patch in the next bot test that we do anyways, just for sanity's sake.

[kim]

Looks reasonable to me. fwiw, there is a smoketest that tests rejection via the client_connected reducer.