Commit 9186f3a
http2: improve nghttp2 error callback
The http2 implementation uses the deprecated function
nghttp2_session_callbacks_set_error_callback, which does not supply an
error code but only an error message. This so far forced node's error
callback to rely on the error message in order to distinguish between
different errors, which is fragile and inefficient.
Use the newer nghttp2_session_callbacks_set_error_callback2 function
instead, which is not deprecated and which provides the exact error code
to node's error callback.
PR-URL: #47840
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>1 parent cf8845d commit 9186f3a
2 files changed
Lines changed: 8 additions & 12 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -437,8 +437,7 @@ Http2Session::Callbacks::Callbacks(bool kHasGetPaddingCallback) { | |||
| 437 | 437 | callbacks_, OnFrameNotSent); | |
| 438 | 438 | nghttp2_session_callbacks_set_on_invalid_header_callback2( | |
| 439 | 439 | callbacks_, OnInvalidHeader); | |
| 440 | - nghttp2_session_callbacks_set_error_callback( | ||
| 441 | - callbacks_, OnNghttpError); | ||
| 440 | + nghttp2_session_callbacks_set_error_callback2(callbacks_, OnNghttpError); | ||
| 442 | 441 | nghttp2_session_callbacks_set_send_data_callback( | |
| 443 | 442 | callbacks_, OnSendData); | |
| 444 | 443 | nghttp2_session_callbacks_set_on_invalid_frame_recv_callback( | |
@@ -1257,21 +1256,18 @@ ssize_t Http2Session::OnSelectPadding(nghttp2_session* handle, | |||
| 1257 | 1256 | return padding; | |
| 1258 | 1257 | } | |
| 1259 | 1258 | ||
| 1260 | - #define BAD_PEER_MESSAGE "Remote peer returned unexpected data while we " \ | ||
| 1261 | - "expected SETTINGS frame. Perhaps, peer does not " \ | ||
| 1262 | - "support HTTP/2 properly." | ||
| 1263 | - | ||
| 1264 | 1259 | // We use this currently to determine when an attempt is made to use the http2 | |
| 1265 | 1260 | // protocol with a non-http2 peer. | |
| 1266 | 1261 | int Http2Session::OnNghttpError(nghttp2_session* handle, | |
| 1262 | + int lib_error_code, | ||
| 1267 | 1263 | const char* message, | |
| 1268 | 1264 | size_t len, | |
| 1269 | 1265 | void* user_data) { | |
| 1270 | 1266 | // Unfortunately, this is currently the only way for us to know if | |
| 1271 | 1267 | // the session errored because the peer is not an http2 peer. | |
| 1272 | 1268 | Http2Session* session = static_cast<Http2Session*>(user_data); | |
| 1273 | 1269 | Debug(session, "Error '%s'", message); | |
| 1274 | - if (strncmp(message, BAD_PEER_MESSAGE, len) == 0) { | ||
| 1270 | + if (lib_error_code == NGHTTP2_ERR_SETTINGS_EXPECTED) { | ||
| 1275 | 1271 | Environment* env = session->env(); | |
| 1276 | 1272 | Isolate* isolate = env->isolate(); | |
| 1277 | 1273 | HandleScope scope(isolate); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -843,11 +843,11 @@ class Http2Session : public AsyncWrap, | |||
| 843 | 843 | const nghttp2_frame* frame, | |
| 844 | 844 | size_t maxPayloadLen, | |
| 845 | 845 | void* user_data); | |
| 846 | - static int OnNghttpError( | ||
| 847 | - nghttp2_session* session, | ||
| 848 | - const char* message, | ||
| 849 | - size_t len, | ||
| 850 | - void* user_data); | ||
| 846 | + static int OnNghttpError(nghttp2_session* session, | ||
| 847 | + int lib_error_code, | ||
| 848 | + const char* message, | ||
| 849 | + size_t len, | ||
| 850 | + void* user_data); | ||
| 851 | 851 | static int OnSendData( | |
| 852 | 852 | nghttp2_session* session, | |
| 853 | 853 | nghttp2_frame* frame, | |
0 commit comments