Skip to content

Commit

Permalink
http_parser: revert d77c3bf
Browse files Browse the repository at this point in the history
As noted in #5555 the changes to
http_parser to use `MakeCallback` have caused a regression that stops
Throws from propagating inside of the http client libs.

A test and more information can be found at https://gist.github.com/TheAlphaNerd/6615a27684deb682dfe7

This revert is being applied directly to v5.x
As such will have no PR-URL.

Fix: #5555
  • Loading branch information
Myles Borins committed Mar 3, 2016
1 parent 01c331e commit 38e1344
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 22 deletions.
1 change: 0 additions & 1 deletion src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ namespace node {
V(FSREQWRAP) \
V(GETADDRINFOREQWRAP) \
V(GETNAMEINFOREQWRAP) \
V(HTTPPARSER) \
V(JSSTREAM) \
V(PIPEWRAP) \
V(PIPECONNECTWRAP) \
Expand Down
27 changes: 9 additions & 18 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
#include "node_buffer.h"
#include "node_http_parser.h"

#include "async-wrap.h"
#include "async-wrap-inl.h"
#include "base-object.h"
#include "base-object-inl.h"
#include "env.h"
#include "env-inl.h"
#include "stream_base.h"
Expand Down Expand Up @@ -147,10 +147,10 @@ struct StringPtr {
};


class Parser : public AsyncWrap {
class Parser : public BaseObject {
public:
Parser(Environment* env, Local<Object> wrap, enum http_parser_type type)
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTPPARSER),
: BaseObject(env, wrap),
current_buffer_len_(0),
current_buffer_data_(nullptr) {
Wrap(object(), this);
Expand All @@ -164,11 +164,6 @@ class Parser : public AsyncWrap {
}


size_t self_size() const override {
return sizeof(*this);
}


HTTP_CB(on_message_begin) {
num_fields_ = num_values_ = 0;
url_.Reset();
Expand Down Expand Up @@ -290,10 +285,8 @@ class Parser : public AsyncWrap {

argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);

Environment::AsyncCallbackScope callback_scope(env());

Local<Value> head_response =
MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);

if (head_response.IsEmpty()) {
got_exception_ = true;
Expand Down Expand Up @@ -328,7 +321,7 @@ class Parser : public AsyncWrap {
Integer::NewFromUnsigned(env()->isolate(), length)
};

Local<Value> r = MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
Local<Value> r = cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);

if (r.IsEmpty()) {
got_exception_ = true;
Expand All @@ -351,9 +344,7 @@ class Parser : public AsyncWrap {
if (!cb->IsFunction())
return 0;

Environment::AsyncCallbackScope callback_scope(env());

Local<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
Local<Value> r = cb.As<Function>()->Call(obj, 0, nullptr);

if (r.IsEmpty()) {
got_exception_ = true;
Expand Down Expand Up @@ -593,7 +584,7 @@ class Parser : public AsyncWrap {
parser->current_buffer_len_ = nread;
parser->current_buffer_data_ = buf->base;

parser->MakeCallback(cb.As<Function>(), 1, &ret);
cb.As<Function>()->Call(obj, 1, &ret);

parser->current_buffer_len_ = 0;
parser->current_buffer_data_ = nullptr;
Expand Down Expand Up @@ -680,7 +671,7 @@ class Parser : public AsyncWrap {
url_.ToString(env())
};

Local<Value> r = MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
Local<Value> r = cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);

if (r.IsEmpty())
got_exception_ = true;
Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-async-wrap-check-providers.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const tls = require('tls');
const zlib = require('zlib');
const ChildProcess = require('child_process').ChildProcess;
const StreamWrap = require('_stream_wrap').StreamWrap;
const HTTPParser = process.binding('http_parser').HTTPParser;
const async_wrap = process.binding('async_wrap');
const pkeys = Object.keys(async_wrap.Providers);

Expand Down Expand Up @@ -107,8 +106,6 @@ zlib.createGzip();

new ChildProcess();

new HTTPParser(HTTPParser.REQUEST);

process.on('exit', function() {
if (keyList.length !== 0) {
process._rawDebug('Not all keys have been used:');
Expand Down

0 comments on commit 38e1344

Please sign in to comment.