Skip to content

Commit 0672c24

Browse files
addaleaxaddaleax
committed
src: pass along errors from http2 object creation
PR-URL: #25822 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
1 parent e3fd752 commit 0672c24

File tree

2 files changed

+87
-62
lines changed

2 files changed

+87
-62
lines changed

‎src/node_http2.cc

+71-54
Original file line numberDiff line numberDiff line change
@@ -228,27 +228,18 @@ void Http2Session::Http2Settings::Init() {
228228
count_ = n;
229229
}
230230

231-
Http2Session::Http2Settings::Http2Settings(Environment* env,
232-
Http2Session* session, uint64_t start_time)
233-
: AsyncWrap(env,
234-
env->http2settings_constructor_template()
235-
->NewInstance(env->context())
236-
.ToLocalChecked(),
237-
PROVIDER_HTTP2SETTINGS),
238-
session_(session),
239-
startTime_(start_time) {
240-
Init();
241-
}
242-
243-
244-
Http2Session::Http2Settings::Http2Settings(Environment* env)
245-
: Http2Settings(env, nullptr, 0) {}
246-
247231
// The Http2Settings class is used to configure a SETTINGS frame that is
248232
// to be sent to the connected peer. The settings are set using a TypedArray
249233
// that is shared with the JavaScript side.
250-
Http2Session::Http2Settings::Http2Settings(Http2Session* session)
251-
: Http2Settings(session->env(), session, uv_hrtime()) {}
234+
Http2Session::Http2Settings::Http2Settings(Environment* env,
235+
Http2Session* session,
236+
Local<Object> obj,
237+
uint64_t start_time)
238+
: AsyncWrap(env, obj, PROVIDER_HTTP2SETTINGS),
239+
session_(session),
240+
startTime_(start_time) {
241+
Init();
242+
}
252243

253244
// Generates a Buffer that contains the serialized payload of a SETTINGS
254245
// frame. This can be used, for instance, to create the Base64-encoded
@@ -918,13 +909,14 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
918909
// The common case is that we're creating a new stream. The less likely
919910
// case is that we're receiving a set of trailers
920911
if (LIKELY(stream == nullptr)) {
921-
if (UNLIKELY(!session->CanAddStream())) {
912+
if (UNLIKELY(!session->CanAddStream() ||
913+
Http2Stream::New(session, id, frame->headers.cat) ==
914+
nullptr)) {
922915
// Too many concurrent streams being opened
923916
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
924917
NGHTTP2_ENHANCE_YOUR_CALM);
925918
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
926919
}
927-
new Http2Stream(session, id, frame->headers.cat);
928920
} else if (!stream->IsDestroyed()) {
929921
stream->StartHeaders(frame->headers.cat);
930922
}
@@ -1771,7 +1763,7 @@ Http2Stream* Http2Session::SubmitRequest(
17711763
*ret = nghttp2_submit_request(session_, prispec, nva, len, *prov, nullptr);
17721764
CHECK_NE(*ret, NGHTTP2_ERR_NOMEM);
17731765
if (LIKELY(*ret > 0))
1774-
stream = new Http2Stream(this, *ret, NGHTTP2_HCAT_HEADERS, options);
1766+
stream = Http2Stream::New(this, *ret, NGHTTP2_HCAT_HEADERS, options);
17751767
return stream;
17761768
}
17771769

@@ -1857,20 +1849,30 @@ void Http2Session::Consume(Local<External> external) {
18571849
Debug(this, "i/o stream consumed");
18581850
}
18591851

1860-
1861-
Http2Stream::Http2Stream(
1862-
Http2Session* session,
1863-
int32_t id,
1864-
nghttp2_headers_category category,
1865-
int options) : AsyncWrap(session->env(),
1866-
session->env()->http2stream_constructor_template()
1867-
->NewInstance(session->env()->context())
1868-
.ToLocalChecked(),
1869-
AsyncWrap::PROVIDER_HTTP2STREAM),
1870-
StreamBase(session->env()),
1871-
session_(session),
1872-
id_(id),
1873-
current_headers_category_(category) {
1852+
Http2Stream* Http2Stream::New(Http2Session* session,
1853+
int32_t id,
1854+
nghttp2_headers_category category,
1855+
int options) {
1856+
Local<Object> obj;
1857+
if (!session->env()
1858+
->http2stream_constructor_template()
1859+
->NewInstance(session->env()->context())
1860+
.ToLocal(&obj)) {
1861+
return nullptr;
1862+
}
1863+
return new Http2Stream(session, obj, id, category, options);
1864+
}
1865+
1866+
Http2Stream::Http2Stream(Http2Session* session,
1867+
Local<Object> obj,
1868+
int32_t id,
1869+
nghttp2_headers_category category,
1870+
int options)
1871+
: AsyncWrap(session->env(), obj, AsyncWrap::PROVIDER_HTTP2STREAM),
1872+
StreamBase(session->env()),
1873+
session_(session),
1874+
id_(id),
1875+
current_headers_category_(category) {
18741876
MakeWeak();
18751877
statistics_.start_time = uv_hrtime();
18761878

@@ -2113,7 +2115,7 @@ Http2Stream* Http2Stream::SubmitPushPromise(nghttp2_nv* nva,
21132115
CHECK_NE(*ret, NGHTTP2_ERR_NOMEM);
21142116
Http2Stream* stream = nullptr;
21152117
if (*ret > 0)
2116-
stream = new Http2Stream(session_, *ret, NGHTTP2_HCAT_HEADERS, options);
2118+
stream = Http2Stream::New(session_, *ret, NGHTTP2_HCAT_HEADERS, options);
21172119

21182120
return stream;
21192121
}
@@ -2335,7 +2337,14 @@ void HttpErrorString(const FunctionCallbackInfo<Value>& args) {
23352337
// output for an HTTP2-Settings header field.
23362338
void PackSettings(const FunctionCallbackInfo<Value>& args) {
23372339
Environment* env = Environment::GetCurrent(args);
2338-
Http2Session::Http2Settings settings(env);
2340+
// TODO(addaleax): We should not be creating a full AsyncWrap for this.
2341+
Local<Object> obj;
2342+
if (!env->http2settings_constructor_template()
2343+
->NewInstance(env->context())
2344+
.ToLocal(&obj)) {
2345+
return;
2346+
}
2347+
Http2Session::Http2Settings settings(env, nullptr, obj);
23392348
args.GetReturnValue().Set(settings.Pack());
23402349
}
23412350

@@ -2464,7 +2473,7 @@ void Http2Session::Request(const FunctionCallbackInfo<Value>& args) {
24642473
session->Http2Session::SubmitRequest(*priority, *list, list.length(),
24652474
&ret, options);
24662475

2467-
if (ret <= 0) {
2476+
if (ret <= 0 || stream == nullptr) {
24682477
Debug(session, "could not submit request: %s", nghttp2_strerror(ret));
24692478
return args.GetReturnValue().Set(ret);
24702479
}
@@ -2637,7 +2646,7 @@ void Http2Stream::PushPromise(const FunctionCallbackInfo<Value>& args) {
26372646
int32_t ret = 0;
26382647
Http2Stream* stream = parent->SubmitPushPromise(*list, list.length(),
26392648
&ret, options);
2640-
if (ret <= 0) {
2649+
if (ret <= 0 || stream == nullptr) {
26412650
Debug(parent, "failed to create push stream: %d", ret);
26422651
return args.GetReturnValue().Set(ret);
26432652
}
@@ -2773,9 +2782,15 @@ void Http2Session::Ping(const FunctionCallbackInfo<Value>& args) {
27732782
CHECK_EQ(Buffer::Length(args[0]), 8);
27742783
}
27752784

2776-
Http2Session::Http2Ping* ping = new Http2Ping(session);
2777-
Local<Object> obj = ping->object();
2778-
obj->Set(env->context(), env->ondone_string(), args[1]).FromJust();
2785+
Local<Object> obj;
2786+
if (!env->http2ping_constructor_template()
2787+
->NewInstance(env->context())
2788+
.ToLocal(&obj)) {
2789+
return;
2790+
}
2791+
if (obj->Set(env->context(), env->ondone_string(), args[1]).IsNothing())
2792+
return;
2793+
Http2Session::Http2Ping* ping = new Http2Ping(session, obj);
27792794

27802795
// To prevent abuse, we strictly limit the number of unacknowledged PING
27812796
// frames that may be sent at any given time. This is configurable in the
@@ -2799,10 +2814,17 @@ void Http2Session::Settings(const FunctionCallbackInfo<Value>& args) {
27992814
Http2Session* session;
28002815
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
28012816

2802-
Http2Session::Http2Settings* settings = new Http2Settings(session);
2803-
Local<Object> obj = settings->object();
2804-
obj->Set(env->context(), env->ondone_string(), args[0]).FromJust();
2817+
Local<Object> obj;
2818+
if (!env->http2settings_constructor_template()
2819+
->NewInstance(env->context())
2820+
.ToLocal(&obj)) {
2821+
return;
2822+
}
2823+
if (obj->Set(env->context(), env->ondone_string(), args[0]).IsNothing())
2824+
return;
28052825

2826+
Http2Session::Http2Settings* settings =
2827+
new Http2Settings(session->env(), session, obj, 0);
28062828
if (!session->AddSettings(settings)) {
28072829
settings->Done(false);
28082830
return args.GetReturnValue().Set(false);
@@ -2849,15 +2871,10 @@ bool Http2Session::AddSettings(Http2Session::Http2Settings* settings) {
28492871
return true;
28502872
}
28512873

2852-
Http2Session::Http2Ping::Http2Ping(
2853-
Http2Session* session)
2854-
: AsyncWrap(session->env(),
2855-
session->env()->http2ping_constructor_template()
2856-
->NewInstance(session->env()->context())
2857-
.ToLocalChecked(),
2858-
AsyncWrap::PROVIDER_HTTP2PING),
2859-
session_(session),
2860-
startTime_(uv_hrtime()) { }
2874+
Http2Session::Http2Ping::Http2Ping(Http2Session* session, Local<Object> obj)
2875+
: AsyncWrap(session->env(), obj, AsyncWrap::PROVIDER_HTTP2PING),
2876+
session_(session),
2877+
startTime_(uv_hrtime()) {}
28612878

28622879
void Http2Session::Http2Ping::Send(uint8_t* payload) {
28632880
uint8_t data[8];

‎src/node_http2.h

+16-8
Original file line numberDiff line numberDiff line change
@@ -451,10 +451,11 @@ class Http2StreamListener : public StreamListener {
451451
class Http2Stream : public AsyncWrap,
452452
public StreamBase {
453453
public:
454-
Http2Stream(Http2Session* session,
455-
int32_t id,
456-
nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS,
457-
int options = 0);
454+
static Http2Stream* New(
455+
Http2Session* session,
456+
int32_t id,
457+
nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS,
458+
int options = 0);
458459
~Http2Stream() override;
459460

460461
nghttp2_stream* operator*();
@@ -611,6 +612,12 @@ class Http2Stream : public AsyncWrap,
611612
Statistics statistics_ = {};
612613

613614
private:
615+
Http2Stream(Http2Session* session,
616+
v8::Local<v8::Object> obj,
617+
int32_t id,
618+
nghttp2_headers_category category,
619+
int options);
620+
614621
Http2Session* session_ = nullptr; // The Parent HTTP/2 Session
615622
int32_t id_ = 0; // The Stream Identifier
616623
int32_t code_ = NGHTTP2_NO_ERROR; // The RST_STREAM code (if any)
@@ -1076,7 +1083,7 @@ class Http2StreamPerformanceEntry : public PerformanceEntry {
10761083

10771084
class Http2Session::Http2Ping : public AsyncWrap {
10781085
public:
1079-
explicit Http2Ping(Http2Session* session);
1086+
explicit Http2Ping(Http2Session* session, v8::Local<v8::Object> obj);
10801087

10811088
void MemoryInfo(MemoryTracker* tracker) const override {
10821089
tracker->TrackField("session", session_);
@@ -1100,8 +1107,10 @@ class Http2Session::Http2Ping : public AsyncWrap {
11001107
// structs.
11011108
class Http2Session::Http2Settings : public AsyncWrap {
11021109
public:
1103-
explicit Http2Settings(Environment* env);
1104-
explicit Http2Settings(Http2Session* session);
1110+
Http2Settings(Environment* env,
1111+
Http2Session* session,
1112+
v8::Local<v8::Object> obj,
1113+
uint64_t start_time = uv_hrtime());
11051114

11061115
void MemoryInfo(MemoryTracker* tracker) const override {
11071116
tracker->TrackField("session", session_);
@@ -1125,7 +1134,6 @@ class Http2Session::Http2Settings : public AsyncWrap {
11251134
get_setting fn);
11261135

11271136
private:
1128-
Http2Settings(Environment* env, Http2Session* session, uint64_t start_time);
11291137
void Init();
11301138
Http2Session* session_;
11311139
uint64_t startTime_;

0 commit comments

Comments
 (0)