Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngtcp2: Return code for QUIC Connect error #4754

Closed
wants to merge 1 commit into from
Closed

ngtcp2: Return code for QUIC Connect error #4754

wants to merge 1 commit into from

Conversation

emilengler
Copy link
Contributor

Removes some todos.
Adds init code 96 and 97 for Quic (SSL) initialization errors

@jay jay added libcurl API HTTP/3 h3 or quic related labels Dec 24, 2019
@jay
Copy link
Member

jay commented Dec 24, 2019

I agree a change is needed but I would use a single error code like CURLE_QUIC_CONNECT_ERROR (in the spirit of CURLE_SSL_CONNECT_ERROR) in place of all the CURLE_FAILED_INIT in that file. The INIT error is intended for library initialization errors. This will remain open to gather feedback. It's the holidays and we're in a feature freeze so regardless this won't move forward right away.

@emilengler
Copy link
Contributor Author

@jay I was also a bit skeptical as SSL is included into Quic...
Ok, I will change it the days.

Have a nice Christmas and a happy new year ;)

@emilengler
Copy link
Contributor Author

@jay Done

@jay
Copy link
Member

jay commented Dec 26, 2019

Thanks, Merry Christmas to you as well.

include/curl/curl.h Outdated Show resolved Hide resolved
Copy link
Member

@jay jay left a comment

Choose a reason for hiding this comment

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

you can change the rest of the CURLE_FAILED_INIT in that file to CURLE_QUIC_CONNECT_ERROR

@jay
Copy link
Member

jay commented Dec 26, 2019

CIs are failing:

strerror.c: In function ‘curl_easy_strerror’:
strerror.c:60:3: error: enumeration value ‘CURLE_QUIC_CONNECT_ERROR’ not handled in switch [-Werror=switch]
switch(error) {
^~~~~~

also you'll have to add it to
docs/libcurl/libcurl-errors.3
docs/libcurl/symbols-in-versions
packages/OS400/curl.inc.in

@emilengler emilengler changed the title ngtcp2: Better return codes for QUIC (SSL) init errors ngtcp2: Return code for QUIC Connect error Dec 26, 2019
@emilengler
Copy link
Contributor Author

you can change the rest of the CURLE_FAILED_INIT in that file to CURLE_QUIC_CONNECT_ERROR

Done

@emilengler
Copy link
Contributor Author

packages/OS400/curl.inc.in
I'm not really sure about that file, not all error codes are included there. e.g CURLE_HTTP3

jay added a commit that referenced this pull request Dec 26, 2019
@jay jay added the feature-window A merge of this requires an open feature window label Dec 26, 2019
@jay
Copy link
Member

jay commented Dec 26, 2019

I'm not really sure about that file, not all error codes are included there. e.g CURLE_HTTP3

Yes they were missing. I've added them in master, rebased your branch on that and then added CURLE_QUIC_CONNECT_ERROR to os400 as well as a few other changes in description. I have just force-pushed them to your branch.

@emilengler
Copy link
Contributor Author

@jay Thanks

@bagder
Copy link
Member

bagder commented Jan 3, 2020

I like the new error code. This PR needs a rebase.

@emilengler
Copy link
Contributor Author

@bagder Rebase done

@jay jay closed this in cbb5429 Jan 12, 2020
@jay
Copy link
Member

jay commented Jan 12, 2020

Thanks

@jay jay removed the feature-window A merge of this requires an open feature window label Jan 12, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
HTTP/3 h3 or quic related libcurl API
Development

Successfully merging this pull request may close these issues.

None yet

3 participants