Page MenuHomeWildfire Games

Show a more helpful message upon an empty response from mod.io
ClosedPublic

Authored by elexis on Aug 9 2018, 6:43 PM.

Details

Summary

If the curl download from mod.io fails, it will throw an error message failing to parse the empty response as JSON:

But curl has error codes and strings for all the possible connection errors and there is code to catch the error codes.
But only with this bugfix the code becomes active:

This confusing error was reported on the forums too and the bugfix may help avoid unneeded reports:
https://wildfiregames.com/forum/index.php?/topic/24718-mod-downloader-is-not-working/&tab=comments#comment-360171

Test Plan

Disconnect and try to Download some mods.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

smiley created this revision.Aug 9 2018, 6:43 PM
elexis added a subscriber: elexis.Aug 9 2018, 6:49 PM

To test if D1600 actually works, I was wondering if we can't catch the CURL error code. It might also be slightly more helpful than the nothing-received test.
Vladislav mentioned that different versions might have different error codes, but I assume we can pass on the CURL error string.
https://curl.haxx.se/libcurl/c/libcurl-errors.html

Vulcan added a subscriber: Vulcan.Aug 9 2018, 6:53 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/differential/714/display/redirect

One can obtain a CURL error string using https://curl.haxx.se/libcurl/c/CURLOPT_ERRORBUFFER.html but I didn't know what @vladislavbelov meant to say about that function.

In D1608#64376, @elexis wrote:

To test if D1600 actually works, I was wondering if we can't catch the CURL error code. It might also be slightly more helpful than the nothing-received test.
Vladislav mentioned that different versions might have different error codes, but I assume we can pass on the CURL error string.
https://curl.haxx.se/libcurl/c/libcurl-errors.html

I meant not codes, but strings. Because codes are much more stable. So we can't translate strings from the third party library.

smiley added a comment.EditedAug 11 2018, 10:55 AM

Some things I found out after searching around https://curl.haxx.se/libcurl/c/ .
I'm not sure I understood it correctly or not. So, inline comments maybe wrong.
But if they are true, an alternative (maybe hacky) solution would be to send a status check request before the actual request.

Edit: https://curl.haxx.se/mail/lib-2015-01/0213.html also seems to do it in a similiar fashion.

source/ps/ModIo.cpp
213 ↗(On Diff #6850)

From my investigation, this returns the status of how adding the easy_handle to multi interface went.

376 ↗(On Diff #6850)

from https://curl.haxx.se/libcurl/c/curl_multi_perform.html

Problems still might have occurred on individual transfers even when this function returns CURLM_OK

So I suppose this cant be used to check connection status unlike easy_perform.

smiley added inline comments.Aug 15 2018, 12:22 PM
source/ps/ModIo.cpp
403 ↗(On Diff #6850)

Apparently I had not gone through everything. AFAIK, the https error can be caught here.

elexis commandeered this revision.Sep 23 2018, 1:45 AM
elexis added a reviewer: smiley.
elexis added inline comments.
source/ps/ModIo.cpp
403 ↗(On Diff #6850)

The message->msg == CURLMSG_DONE || message->easy_handle == m_Curl condition must be removed, since it's the only message that carries the error code, issue from rP21759, from https://gitlab.com/na-Itms/0ad/commit/cdc324f7f57bf3098c196c59a92718b1ab4d1e87

All messages that arrive here relate to the original request, because there is at most one request at a time with this code.

elexis edited the summary of this revision. (Show Details)Sep 23 2018, 1:53 AM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 23 2018, 2:07 AM
This revision was automatically updated to reflect the committed changes.