Fix WinHTTP initialization timing to prevent error 12007 #1321
Conversation
jpnurmi
left a comment
jpnurmi
left a comment
There was a problem hiding this comment.
Any idea if this works for Xbox One, too?
Maybe we could use:
-
if(XBOX)in CMakeLine 14 in 70f38a2
-
#ifdef SENTRY_PLATFORM_XBOXin Csentry-native/include/sentry.h
Line 40 in 70f38a2
?
|
@jpnurmi How can I address "Danger" CI Issue? |
Resolve an issue in Microsoft GDK titles where WinHttpSendRequest intermittently failed with error code 12007 in Release Package builds. This fix delays the initialization of the HTTP session until the network stack is fully ready and the first HTTP request is made https://learn.microsoft.com/pl-pl/gaming/gdk/docs/features/console/networking/initialization-connectivity-networking
vaind
left a comment
vaind
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I've got some concerns listed below:
| @@ -0,0 +1,64 @@ | |||
| #include "sentry_boot.h" | |||
There was a problem hiding this comment.
I think we should keep the contents here in the *winhttp.c file instead of creating a new one. So far, the mapping is file=transport. This would break that by only extending what's needed for winhttp transport.
There was a problem hiding this comment.
I've created dedicated file because xnetworking requires c++ compiler. Having that in mind, do you still think that it should be merged to winhttp.c?
There was a problem hiding this comment.
Then it's probably good as is, I'll leave this up to the maintainers
There was a problem hiding this comment.
Also, it may be worthwile to just move this PR to sentry-xbox repository. Then this being a cpp file would be a non-issue
| DWORD result = WaitForSingleObjectEx( | ||
| network_initialized_event, INFINITE, FALSE); |
There was a problem hiding this comment.
This may block indefinitely. Is that the right thing to do? I guess it's alright if we're on the background thread, but might be something to keep in mind in relation to #1316 - at the very least, add a TODO mentioning that issue
There was a problem hiding this comment.
Similar to what we do for Switch in
sentry-native/src/transports/sentry_transport_curl.c
Lines 192 to 196 in d0d732e
| sentry__transport_ensure_network_initialized(); | ||
| sentry__winhttp_session_start(state); |
There was a problem hiding this comment.
Just to make sure - are both of these safe to call from a background thread?
| winhttp_bgworker_state_t *state = (winhttp_bgworker_state_t *)_state; | ||
|
|
||
| #ifdef SENTRY_PLATFORM_XBOX | ||
| if (!state->session) { |
There was a problem hiding this comment.
We're only checking session once, however, the suspend/resume cycle would influence the connectivity. Do we need to ensure connection every time we make a request instead?
|
Replaced by https://github.com/getsentry/sentry-xbox/pull/35 |
5 participants
Resolve an issue in Microsoft GDK titles where WinHttpSendRequest intermittently failed with error code 12007 in Release Package builds. This fix delays the initialization of the HTTP session until the network stack is fully ready and the first HTTP request is made
https://learn.microsoft.com/pl-pl/gaming/gdk/docs/features/console/networking/initialization-connectivity-networking