Skip to content

node 4x retry #74

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.

Sign up for GitHub

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

Closed
erulabs wants to merge 3 commits into from
Closed

node 4x retry #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eafb4b4
node 4x retry
erulabs Sep 28, 2015
5cb40dd
all tests pass except leak
erulabs Sep 28, 2015
c6a7167
very very close. Missing something on error handeling for unknown pro…
erulabs Sep 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
language: node_js
node_js:
- "0.8"
- "0.10"
- "0.11"
- iojs
before_install:
- "sudo apt-get install libicu-dev"
# Workaround for a permissions issue with Travis virtual machine images
# that breaks Python's multiprocessing:
# https://github.com/travis-ci/travis-cookbooks/issues/155
- sudo rm -rf /dev/shm
- sudo ln -s /run/shm /dev/shm
- npm i -g [email protected]
- "4.1.1"
sudo: false
env:
- CXX="g++-4.8"
addons:
apt:
packages:
- libicu-dev
- g++-4.8
- gcc-4.8
sources:
- ubuntu-toolchain-r-test
2 changes: 1 addition & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var UIDNA_ALLOW_UNASSIGNED = 1
var UIDNA_USE_STD3_RULES = 2

try {
var bindings = require('bindings')('node_stringprep.node')
var bindings = require('./node_modules/bindings/bindings.js')('node_stringprep.node')
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

whoa, no, why was this changed?

} catch (ex) {
log(
'Cannot load StringPrep-' +
Expand Down
118 changes: 59 additions & 59 deletions node-stringprep.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ class UnknownProfileException : public std::exception {
// protect constructor from GC
static Persistent<FunctionTemplate> stringprep_constructor;

class StringPrep : public ObjectWrap {
class StringPrep : public Nan::ObjectWrap {
public:
static void Initialize(Handle<Object> target)
{
NanScope();
Local<FunctionTemplate> t = NanNew<FunctionTemplate>(New);
NanAssignPersistent(stringprep_constructor, t);
Nan::HandleScope scope;
Local<FunctionTemplate> t = Nan::New<FunctionTemplate>(New);
//stringprep_constructor.Reset( t);
t->InstanceTemplate()->SetInternalFieldCount(1);
NODE_SET_PROTOTYPE_METHOD(t, "prepare", Prepare);
Nan::SetPrototypeMethod(t, "prepare", Prepare);

target->Set(NanNew<String>("StringPrep"), t->GetFunction());
target->Set(Nan::New<String>("StringPrep").ToLocalChecked(), t->GetFunction());
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

use Nan::Set() for this

}

bool good() const
Expand All @@ -43,39 +43,39 @@ class StringPrep : public ObjectWrap {

static NAN_METHOD(New)
{
NanScope();
Nan::HandleScope scope;
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

remove


if (args.Length() >= 1 && args[0]->IsString())
if (info.Length() >= 1 && info[0]->IsString())
{
String::Utf8Value arg0(args[0]->ToString());
String::Utf8Value arg0(info[0]->ToString());
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

Nan::Utf8Value

UStringPrepProfileType profileType;
try
{
profileType = parseProfileType(arg0);
}
catch (UnknownProfileException &)
{
NanThrowTypeError("Unknown StringPrep profile");
NanReturnUndefined();
Nan::ThrowTypeError("Unknown StringPrep profile");
return;
}

StringPrep *self = new StringPrep(profileType);
if (self->good())
{
self->Wrap(args.This());
NanReturnValue(args.This());
self->Wrap(info.This());
info.GetReturnValue().Set(info.This());
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

return - not strictly necessary but you're returning in other branches so be safe and communicate intention to future devs

}
else
{
const char* err = self->errorName();
delete self;
NanThrowError(err);
NanReturnUndefined();
Nan::ThrowError(err);
return;
}
}
else {
NanThrowTypeError("Bad argument.");
NanReturnUndefined();
Nan::ThrowTypeError("Bad argument.");
return;
}
}

Expand All @@ -97,17 +97,17 @@ class StringPrep : public ObjectWrap {

static NAN_METHOD(Prepare)
{
NanScope();
Nan::HandleScope scope;
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

remove


if (args.Length() >= 1 && args[0]->IsString())
if (info.Length() >= 1 && info[0]->IsString())
{
StringPrep *self = ObjectWrap::Unwrap<StringPrep>(args.This());
String::Value arg0(args[0]->ToString());
NanReturnValue(self->prepare(arg0));
StringPrep *self = Nan::ObjectWrap::Unwrap<StringPrep>(info.This());
String::Value arg0(info[0]->ToString());
info.GetReturnValue().Set(self->prepare(arg0));
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

return

}
else {
NanThrowTypeError("Bad argument.");
NanReturnUndefined();
Nan::ThrowTypeError("Bad argument.");
return;
}
}

Expand All @@ -117,7 +117,7 @@ class StringPrep : public ObjectWrap {
UChar *dest = NULL;
while(!dest)
{
error = U_ZERO_ERROR;
UErrorCode error = U_ZERO_ERROR;
dest = new UChar[destLen];
size_t w = usprep_prepare(profile,
*str, str.length(),
Expand All @@ -131,20 +131,19 @@ class StringPrep : public ObjectWrap {
delete[] dest;
dest = NULL;
}
else if (!good())
else if (!U_SUCCESS(error))
{
// other error, just bail out
delete[] dest;
NanThrowError(errorName());
return NanUndefined();
Nan::ThrowError(u_errorName(error));
return Nan::Undefined();
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

this is the scope.Escape(Nan::Undefined())

}
else
destLen = w;
}

Local<String> result = NanNew<String>(dest, destLen);
delete[] dest;
return result;
return Nan::New<String>(dest, destLen).ToLocalChecked();
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

and scope.Escape(Nan::New<String>(dest, destLen).ToLocalChecked())

}

private:
Expand Down Expand Up @@ -192,12 +191,12 @@ class StringPrep : public ObjectWrap {

NAN_METHOD(ToUnicode)
{
NanScope();
Nan::HandleScope scope;
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

remove


if (args.Length() >= 2 && args[0]->IsString() && args[1]->IsInt32())
if (info.Length() >= 2 && info[0]->IsString() && info[1]->IsInt32())
{
String::Value str(args[0]->ToString());
int32_t options = args[1]->ToInt32()->Value();
String::Value str(info[0]->ToString());
int32_t options = info[1]->ToInt32()->Value();

// ASCII encoding (xn--*--*) should be longer than Unicode
size_t destLen = str.length() + 1;
Expand All @@ -210,7 +209,7 @@ NAN_METHOD(ToUnicode)
dest, destLen,
options,
NULL, &error);

if (error == U_BUFFER_OVERFLOW_ERROR)
{
// retry with a dest buffer twice as large
Expand All @@ -222,31 +221,31 @@ NAN_METHOD(ToUnicode)
{
// other error, just bail out
delete[] dest;
NanThrowError(u_errorName(error));
NanReturnUndefined();
Nan::ThrowError(u_errorName(error));
return;
}
else
destLen = w;
}

Local<String> result = NanNew<String>(dest, destLen);
Local<String> result = Nan::New<String>(dest, destLen).ToLocalChecked();
delete[] dest;
NanReturnValue(result);
info.GetReturnValue().Set(result);
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

return

}
else {
NanThrowTypeError("Bad argument.");
NanReturnUndefined();
Nan::ThrowTypeError("Bad argument.");
return;
}
}

NAN_METHOD(ToASCII)
{
NanScope();
Nan::HandleScope scope;
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

remove


if (args.Length() >= 2 && args[0]->IsString() && args[1]->IsInt32())
if (info.Length() >= 2 && info[0]->IsString() && info[1]->IsInt32())
{
String::Value str(args[0]->ToString());
int32_t options = args[1]->ToInt32()->Value();
String::Value str(info[0]->ToString());
int32_t options = info[1]->ToInt32()->Value();

// find out length first
UErrorCode error = U_ZERO_ERROR;
Expand All @@ -270,28 +269,29 @@ NAN_METHOD(ToASCII)
{
// other error, just bail out
delete[] dest;
NanThrowError(u_errorName(error));
NanReturnUndefined();
Nan::ThrowError(u_errorName(error));
return;
}

Local<String> result = NanNew<String>(dest, destLen);
Local<String> result = Nan::New<String>(dest, destLen).ToLocalChecked();
delete[] dest;
NanReturnValue(result);
info.GetReturnValue().Set(result);
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

return

}
else {
NanThrowTypeError("Bad argument.");
NanReturnUndefined();
Nan::ThrowTypeError("Bad argument.");
return;
}
}

/*** Initialization ***/

extern "C" void init(Handle<Object> target)
{
NanScope();
StringPrep::Initialize(target);
NODE_SET_METHOD(target, "toUnicode", ToUnicode);
NODE_SET_METHOD(target, "toASCII", ToASCII);
extern "C" {
static void init (Handle<Object> target)
Copy link
Contributor

@rvagg rvagg Sep 29, 2015

Choose a reason for hiding this comment

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

Handle will end up being a problem here (it's being deprecated so you'll end up with errors in a version of Node or two), opt for declaring this as NAN_MODULE_INIT(init) anyway and you'll be safe

{
Nan::HandleScope scope;
StringPrep::Initialize(target);
Nan::SetMethod(target, "toUnicode", ToUnicode);
Nan::SetMethod(target, "toASCII", ToASCII);
}
NODE_MODULE(node_stringprep, init)
}

NODE_MODULE(node_stringprep, init)
13 changes: 7 additions & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,17 @@
},
"dependencies": {
"bindings": "~1.2.1",
"debug": "~2.0.0",
"nan": "~1.8.4"
"debug": "~2.2.0",
"iconv-lite": "^0.4.12",
"nan": "~2.0.9"
},
"devDependencies": {
"grunt": "~0.4.2",
"grunt-cli": "^0.1.13",
"grunt-contrib-jshint": "~0.7.2",
"grunt-mocha-cli": "~1.3.0",
"proxyquire": "~0.5.2",
"should": "~2.1.1"
"grunt-contrib-jshint": "~0.11.3",
"grunt-mocha-cli": "~1.14.0",
"proxyquire": "~1.7.2",
"should": "~7.1.0"
},
"repository": {
"type": "git",
Expand Down
14 changes: 7 additions & 7 deletions test/fallback.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('Should use JS fallbacks for StringPrep', function() {
var prep = new StringPrep('cRaZYcASE')
try {
prep.prepare('UPPERCASE')
done('Should have thrown error')
done(new Error('Should have thrown error'))
} catch (e) {
e.message.should.equal(prep.UNKNOWN_PROFILE_TYPE)
done()
Expand Down Expand Up @@ -58,7 +58,7 @@ describe('Should use JS fallbacks for StringPrep', function() {
try {
prep.prepare('UPPERCASE')
isDone = true
done('Should have thrown error')
done(new Error('Should have thrown error for profile ' + profile))
} catch (e) {
e.message.should.equal(prep.UNHANDLED_FALLBACK)
}
Expand All @@ -69,35 +69,35 @@ describe('Should use JS fallbacks for StringPrep', function() {
})

describe('Can disable fallbacks', function() {

var StringPrep = proxyquire('../index', { 'bindings': null }).StringPrep

it('Should allow javascript fallbacks to be disabled', function(done) {
var prep = new StringPrep('nameprep')
try {
prep.disableJsFallbacks()
prep.prepare('UPPERCASE')
done('Should have thrown exception')
done(new Error('Should have thrown exception'))
} catch (e) {
e.message.should.equal(prep.LIBICU_NOT_AVAILABLE)
done()
}
})

it('Should allow javascript fallbacks to be re-enabled', function(done) {
var prep = new StringPrep('nameprep')
try {
prep.disableJsFallbacks()
prep.prepare('UPPERCASE')
done('Should have thrown exception')
done(new Error('Should have thrown exception'))
} catch (e) {
e.message.should.equal(prep.LIBICU_NOT_AVAILABLE)
prep.enableJsFallbacks()
prep.prepare('UPPERCASE')
done()
}
})

})

describe('\'isNative\' method test', function() {
Expand Down