Fix empty exception messages, lost log lines, and unbounded curl requests in TitleHelper - #12
Open
Ashnar2602 wants to merge 3 commits into
Open
Conversation
full_message_, the field backing what(), was declared in XGDException.h but never assigned anywhere. log_error() built the real error description into a local error_message and only ever wrote it to std::cerr, so every caller that catches an XGDException and logs e.what() got an empty string instead of the actual error detail. Assign full_message_ in log_error() so what() returns the same text that gets logged.
The destructor wrote directly to std::cerr instead of routing through the same flush path as an explicit "<< Endl". Several call sites in this codebase (e.g. the catch blocks in InputHelper.cpp) build a log line ending in "\n" rather than an explicit Endl and rely on the destructor to flush it, so those lines never reached whatever sink Endl is actually wired to. Route the destructor through operator<<(Endl) so buffered lines flush the same way regardless of whether Endl was used explicitly.
internet_connected(), set_ogx_titles_online(), unity_query(), and unity_get_title_icon() all call curl_easy_perform() without setting CURLOPT_CONNECTTIMEOUT or CURLOPT_TIMEOUT. On a slow or unreachable network, any of these calls can block indefinitely, stalling the whole operation with no indication of what is happening. Add a connect timeout and an overall timeout to all four requests (3s/5s for the plain connectivity check, 5s/15s for the others) so a bad network degrades gracefully instead of hanging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three small, independent bug fixes found while working with this codebase:
XGDException::what()always returned an empty string.full_message_, the field backingwhat(), was declared inXGDException.hbut never assigned anywhere.log_error()built the real error text into a local variable and only ever sent it tostd::cerr, so anycatch (const XGDException& e)block that logse.what()got nothing useful. Fixed by assigningfull_message_inlog_error().XGDLog's destructor bypassed the configured output sink. The destructor wrote straight tostd::cerrinstead of going through the same path as an explicit<< Endl. Several call sites (e.g. the catch blocks inInputHelper.cpp) build a line ending in\nand rely on the destructor to flush it, so those lines never reached whateverEndlis actually wired to (e.g. the GUI's log view). Fixed by routing the destructor throughoperator<<(Endl).No timeouts on
TitleHelper's curl requests.internet_connected(),set_ogx_titles_online(),unity_query(), andunity_get_title_icon()all callcurl_easy_perform()withoutCURLOPT_CONNECTTIMEOUT/CURLOPT_TIMEOUT. On a slow or unreachable network this can hang indefinitely with no feedback. Added reasonable timeouts to all four.Each fix is its own commit. No functional/behavioral changes beyond what's described above.