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

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

Details

Reviewers

lyv

Commits

rP21891: Fix mod.io curl error handling, following rP21759 / D1029 / https://gitlab.

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

Branch

/ps/trunk

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 6330
Build 10503: Vulcan Build	Jenkins
Build 10502: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	0 ms	Jenkins > TestModIo::Unknown Unit Message ("") Error: Expected (std::string(err) == std::string("Failed to parse response as JSON.")), found ("Recieved no data from mod.io API." != "Failed to parse response as JSON.")
	0 ms	Jenkins > TestModIo::Unknown Unit Message ("") Error: Expected (std::string(err) == std::string("Failed to parse response as JSON.")), found ("Recieved no data from mod.io API." != "Failed to parse response as JSON.")
	0 ms	Jenkins > TestAllocators::Unknown Unit Message ("")
	0 ms	Jenkins > TestAllocators::Unknown Unit Message ("")
	0 ms	Jenkins > TestAtlasObjectXML::Unknown Unit Message ("")
		View Full Test Results (2 Failed · 622 Passed)

Event Timeline

lyv created this revision.Aug 9 2018, 6:43 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

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

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

Harbormaster failed remote builds in B6330: Diff 6850!Aug 9 2018, 6:53 PM

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.

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	From my investigation, this returns the status of how adding the easy_handle to multi interface went.
376	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.

lyv added inline comments.Aug 15 2018, 12:22 PM

source/ps/ModIo.cpp
403	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: lyv.

elexis added inline comments.

source/ps/ModIo.cpp
403	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.