Skip to content

always use Curl_1st_fatal instead of Curl_1st_err#20980

Closed
icing wants to merge 1 commit intocurl:masterfrom
icing:always-use-1st-fatal
Closed

always use Curl_1st_fatal instead of Curl_1st_err#20980
icing wants to merge 1 commit intocurl:masterfrom
icing:always-use-1st-fatal

Conversation

@icing
Copy link
Copy Markdown
Contributor

@icing icing commented Mar 18, 2026

Curl_1st_err() does not return the second error if the first result is CURLE_AGAIN. This may cause errors to not become noticeable when they should be.

Replace all use of Curl_1st_err() with Curl_1st_fatal(), which handles CURLE_AGAIN as a not-a-real-error case.

Curl_1st_err() does not return the second error if the first
result is CURLE_AGAIN. This may cause errors to not become
noticeable when they should be.

Replace all use of Curl_1st_err() with Curl_1st_fatal(), which
handles CURLE_AGAIN as a not-a-real-error case.
@bagder bagder requested a review from Copilot March 18, 2026 10:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes Curl_1st_err() and standardizes error-combination logic on Curl_1st_fatal(), fixing cases where a prior CURLE_AGAIN result could mask a subsequent real error (notably in HTTP/3/QUIC egress/expiry paths).

Changes:

  • Replaced remaining Curl_1st_err() call sites with Curl_1st_fatal() in QUIC (quiche/ngtcp2) and core transfer paths.
  • Removed Curl_1st_err() declaration from lib/url.h and its implementation from lib/url.c.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/vquic/curl_quiche.c Uses Curl_1st_fatal() when combining CURLE_AGAIN with egress flush results so real errors aren’t masked.
lib/vquic/curl_ngtcp2.c Uses Curl_1st_fatal() to ensure egress/expiry errors surface even when the main operation yields CURLE_AGAIN.
lib/url.h Removes the Curl_1st_err() prototype; keeps Curl_1st_fatal() as the supported helper.
lib/url.c Removes the Curl_1st_err() implementation, leaving Curl_1st_fatal() as the sole helper.
lib/multi.c Ensures Curl_xfer_write_done() errors aren’t hidden by a prior CURLE_AGAIN.
lib/http.c Ensures late header-write errors aren’t masked by a prior CURLE_AGAIN.
lib/easy.c Ensures pause/unpause helper errors aren’t masked by a prior CURLE_AGAIN.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@bagder bagder closed this in 41aaac6 Mar 18, 2026
dkarpov1970 pushed a commit to dkarpov1970/curl that referenced this pull request Mar 25, 2026
Curl_1st_err() does not return the second error if the first result is
CURLE_AGAIN. This may cause errors to not become noticeable when they
should be.

Replace all use of Curl_1st_err() with Curl_1st_fatal(), which handles
CURLE_AGAIN as a not-a-real-error case.

Closes curl#20980
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants