Skip to content

Commit daf72e9

Browse files
ifreundbapt
authored andcommitted
libpkg: improve curl fetch error reporting
Currently all errors that occur in curl_do_fetch() are reported to the user as "Failed to fetch X: No Error" which is both incorrect and very unhelpful. With this patch, the correct error message from libcurl is reported to the user, for example "Failed to fetch X: Could not resolve hostname" This patch does not fix all the error handling done by the libcurl fetching code, but it does fix the case that seems to be most commonly hit by users. Closes: #2498 Sponsored by: The FreeBSD Foundation
1 parent 503c2e9 commit daf72e9

1 file changed

Lines changed: 34 additions & 18 deletions

File tree

libpkg/fetch_libcurl.c

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,10 @@ int my_trace(CURL *handle, curl_infotype type,
141141
return 0;
142142
}
143143

144+
/* Returns the HTTP response code on success.
145+
* Returns -1 and sets the `error` out parameter on failure. */
144146
static long
145-
curl_do_fetch(struct curl_userdata *data, CURL *cl, struct curl_repodata *cr)
147+
curl_do_fetch(struct curl_userdata *data, CURL *cl, struct curl_repodata *cr, CURLcode *error)
146148
{
147149
char *tmp;
148150
int still_running = 1;
@@ -180,16 +182,21 @@ curl_do_fetch(struct curl_userdata *data, CURL *cl, struct curl_repodata *cr)
180182

181183
while ((msg = curl_multi_info_read(cr->cm, &msg_left))) {
182184
if (msg->msg == CURLMSG_DONE) {
183-
if (msg->data.result == CURLE_ABORTED_BY_CALLBACK)
184-
return (-1); // FIXME: more clear return code?
185185
if (msg->data.result == CURLE_COULDNT_CONNECT
186186
|| msg->data.result == CURLE_COULDNT_RESOLVE_HOST
187-
|| msg->data.result == CURLE_COULDNT_RESOLVE_PROXY)
187+
|| msg->data.result == CURLE_COULDNT_RESOLVE_PROXY) {
188188
pkg_emit_pkg_errno(EPKG_NONETWORK, "curl_do_fetch", NULL);
189+
}
190+
if (msg->data.result != CURLE_OK) {
191+
if (error != NULL) {
192+
*error = msg->data.result;
193+
}
194+
return (-1);
195+
}
189196
CURL *eh = msg->easy_handle;
190-
long rc = 0;
191-
curl_easy_getinfo(eh, CURLINFO_RESPONSE_CODE, &rc);
192-
return (rc);
197+
long response_code = 0;
198+
curl_easy_getinfo(eh, CURLINFO_RESPONSE_CODE, &response_code);
199+
return (response_code);
193200
}
194201
}
195202
return (0);
@@ -233,7 +240,8 @@ http_getmirrors(struct pkg_repo *r, struct curl_repodata *cr)
233240
curl_easy_setopt(cl, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4);
234241
if (ctx.ip == IPV6)
235242
curl_easy_setopt(cl, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V6);
236-
curl_do_fetch(&data, cl, cr);
243+
// FIXME: handle fetch error?
244+
curl_do_fetch(&data, cl, cr, NULL);
237245
fclose(data.fh);
238246
walk = buf;
239247
while ((line = strsep(&walk, "\n\r")) != NULL) {
@@ -499,34 +507,42 @@ curl_fetch(struct pkg_repo *repo, int dest, struct fetch_item *fi)
499507
curl_easy_setopt(cl, CURLOPT_LOW_SPEED_TIME, repo->fetcher->timeout);
500508
}
501509

502-
long rc = curl_do_fetch(&data, cl, cr);
510+
CURLcode fetch_error;
511+
long response_code = curl_do_fetch(&data, cl, cr, &fetch_error);
503512
curl_off_t t;
504513
res = curl_easy_getinfo(cl, CURLINFO_FILETIME_T, &t);
505514
curl_multi_remove_handle(cr->cm, cl);
506515
curl_easy_cleanup(cl);
507-
if (rc == 304) {
516+
if (response_code == -1) {
517+
if (fetch_error == CURLE_ABORTED_BY_CALLBACK) {
518+
retcode = EPKG_CANCEL;
519+
} else {
520+
pkg_emit_error("Failed to fetch %s: %s",
521+
fi->url, curl_easy_strerror(fetch_error));
522+
retcode = EPKG_FATAL;
523+
}
524+
} else if (response_code == 304) {
508525
retcode = EPKG_UPTODATE;
509-
} else if (rc == -1) {
510-
retcode = EPKG_CANCEL;
511-
} else if (!curl_response_is_ok(rc)) {
526+
} else if (!curl_response_is_ok(response_code)) {
512527
--retry;
513528
if (retry <= 0) {
514-
if (rc == 404) {
529+
if (response_code == 404) {
515530
pkg_emit_error("Failed to fetch %s: Not found",
516531
fi->url);
517532
retcode = EPKG_ENOENT;
518533
} else {
519-
pkg_emit_error("Failed to fetch %s: %s",
520-
fi->url, curl_easy_strerror(rc));
534+
pkg_emit_error("Failed to fetch %s: response code %ld",
535+
fi->url, response_code);
521536
retcode = EPKG_FATAL;
522537
}
523-
} else
538+
} else {
524539
goto do_retry;
540+
}
525541
}
526542

527543
if (res == CURLE_OK && t >= 0) {
528544
fi->mtime = t;
529-
} else if (rc != 304 && retcode != EPKG_FATAL &&
545+
} else if (response_code != 304 && retcode != EPKG_FATAL &&
530546
retcode != EPKG_CANCEL && retcode != EPKG_ENOENT) {
531547
pkg_emit_error("Impossible to get the value from Last-Modified"
532548
" HTTP header");

0 commit comments

Comments
 (0)