Update Starscream, for real this time #1659
Conversation
|
|
||
| /// Determines whether a SOCKS proxy is enabled on the underlying request. | ||
| /// Mostly useful for debugging with tools like Charles Proxy. | ||
| var enableSOCKSProxy: Bool { get set } |
There was a problem hiding this comment.
unfortunately v4 of Starscream doesn't support proxying at this time, so I had to remove this.
| return | ||
| } | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + reconnectionInterval) { |
There was a problem hiding this comment.
Is it right to always be doing this on the main queue? Do we have to use main for some reason here?
There was a problem hiding this comment.
That's a reasonable question - this bit was basically moving around some existing code.
I suspect part of the reason it's on main is because we do the initial connection on main by default, but I suspect the bigger reason is because you can't just grab the current queue and dispatch on it the way you could in ObjC.
… getting due to a bug
designatednerd
force-pushed
the
update/no-seriously-starscream
branch
from
76c7581
to
6cf7296
Compare
|
I've poked around a bit on the bug and gotten no response, so for now I'm going to disable the check for the disconnection notification so we can get this out the door. |
So I tried this in #1394 and messed it up something awful, so I took a step back and tried again. It's marginally less awful this time!
There's one place where a test on reconnection is failing because if you voluntarily call
disconnect, it appears you never get any kind of callback that you've disconnected due to this issue with Starscream. I'm gonna see if anyone knows what's going on with that before giving up and updating the code in the tests that's waiting for the confirmation that the disconnection happened before trying to reconnect.