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

Conversation

@emilengler
Copy link
Contributor

emilengler commented Dec 24, 2019

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

@jay

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

emilengler commented Dec 24, 2019

@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 emilengler force-pushed the emilengler:2019-12-quic-init-error-codes branch from 71d0042 to 1ad117d Dec 25, 2019
@emilengler

This comment has been minimized.

Copy link
Contributor Author

emilengler commented Dec 25, 2019

@jay Done

@jay

This comment has been minimized.

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 left a comment

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

@jay

This comment has been minimized.

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 force-pushed the emilengler:2019-12-quic-init-error-codes branch from 1ad117d to d6bf3c9 Dec 26, 2019
@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 emilengler force-pushed the emilengler:2019-12-quic-init-error-codes branch from d6bf3c9 to 3deb9d4 Dec 26, 2019
@emilengler

This comment has been minimized.

Copy link
Contributor Author

emilengler commented Dec 26, 2019

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

Done

@emilengler

This comment has been minimized.

Copy link
Contributor Author

emilengler commented Dec 26, 2019

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

@emilengler emilengler force-pushed the emilengler:2019-12-quic-init-error-codes branch from 3deb9d4 to b06d1d0 Dec 26, 2019
jay added a commit that referenced this pull request Dec 26, 2019
Bug: #4754 (comment)
Reported-by: Emil Engler
jay added a commit to emilengler/curl that referenced this pull request Dec 26, 2019
- Add new error code CURLE_QUIC_CONNECT_ERROR for QUIC connection
  errors.

Prior to this change CURLE_FAILED_INIT was used, but that was not
correct.

Closes curl#4754
@jay jay force-pushed the emilengler:2019-12-quic-init-error-codes branch from b06d1d0 to 41816d7 Dec 26, 2019
@jay

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

emilengler commented Dec 26, 2019

@jay Thanks

jay added a commit to emilengler/curl that referenced this pull request Dec 28, 2019
- Add new error code CURLE_QUIC_CONNECT_ERROR for QUIC connection
  errors.

Prior to this change CURLE_FAILED_INIT was used, but that was not
correct.

Closes curl#4754
@jay jay force-pushed the emilengler:2019-12-quic-init-error-codes branch from 41816d7 to c30bb5b Dec 28, 2019
jay added a commit to emilengler/curl that referenced this pull request Dec 29, 2019
- Add new error code CURLE_QUIC_CONNECT_ERROR for QUIC connection
  errors.

Prior to this change CURLE_FAILED_INIT was used, but that was not
correct.

Closes curl#4754
@jay jay force-pushed the emilengler:2019-12-quic-init-error-codes branch from c30bb5b to 8af02b1 Dec 29, 2019
@bagder

This comment has been minimized.

Copy link
Member

bagder commented Jan 3, 2020

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

@emilengler emilengler force-pushed the emilengler:2019-12-quic-init-error-codes branch from 8af02b1 to 3a1eeea Jan 3, 2020
@emilengler

This comment has been minimized.

Copy link
Contributor Author

emilengler commented Jan 3, 2020

@bagder Rebase done

@jay jay closed this in cbb5429 Jan 12, 2020
@jay

This comment has been minimized.

Copy link
Member

jay commented Jan 12, 2020

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.