Skip to content

Commit

Permalink
cleanup document error handling
Browse files Browse the repository at this point in the history 
avoid using persistent handles for error objects
  • Loading branch information
@defunctzombie
defunctzombie committed Dec 19, 2011
1 parent ad46995 commit 251b18a
Show file tree
Hide file tree
Showing 14 changed files with 112 additions and 504 deletions.
41 changes: 2 additions & 39 deletions spec/spec_html_parser.js
View file
Expand Up @@ -6,45 +6,25 @@ var recoverableErrors = [
code: 23,
message: "htmlParseEntityRef: expecting ';'\n",
level: 2,
file: recoverableFile,
line: 12,
str1: null,
str2: null,
str3: null,
int1: null,
column: 27 },
{ domain: 5,
code: 68,
message: "htmlParseEntityRef: no name\n",
level: 2,
file: recoverableFile,
line: 12,
str1: null,
str2: null,
str3: null,
int1: null,
column: 38 },
{ domain: 5,
code: 23,
message: "htmlParseEntityRef: expecting ';'\n",
level: 2,
file: recoverableFile,
line: 14,
str1: null,
str2: null,
str3: null,
int1: null,
column: 4 },
{ domain: 5,
code: 68,
message: "htmlParseEntityRef: no name\n",
level: 2,
file: recoverableFile,
line: 15,
str1: null,
str2: null,
str3: null,
int1: null,
column: 4 }
];

Expand All @@ -59,32 +39,15 @@ describe('Parsing HTML', function() {
assert.equal('Test HTML document', doc.get('head/title').text());
assert.equal('HTML content!', doc.get('body/span').text());
});

it('can be done by file', function() {
var doc = libxml.parseHtmlFile(filename);
assert.equal('html', doc.root().name());
assert.equal('Test HTML document', doc.get('head/title').text());
assert.equal('HTML content!', doc.get('body/span').text());
});
});

describe('A recoverable parse error when parsing an HTML file', function() {
it('will attach the errors to the document', function() {
var doc = libxml.parseHtmlFile(recoverableFile);
assert.equal(4, doc.errors().length);
assert.deepEqual(recoverableErrors, doc.errors());
});
});

describe('A recoverable parse error when parsing an HTML string', function() {
var str = fs.readFileSync(recoverableFile, 'utf8');

it('will attach the errors to the document', function() {
var doc = libxml.parseHtmlString(str);
assert.equal(4, doc.errors().length);
for (var i in recoverableErrors)
recoverableErrors[i].file = null;
assert.deepEqual(recoverableErrors, doc.errors());
assert.equal(4, doc.errors.length);
assert.deepEqual(recoverableErrors, doc.errors);
});
});

Expand Down
31 changes: 0 additions & 31 deletions spec/spec_xml_parse_errors.js
View file

This file was deleted.

68 changes: 2 additions & 66 deletions spec/spec_xml_parser.js
View file
Expand Up @@ -17,67 +17,6 @@ describe('Parsing XML', function() {
assert.equal('with content!', doc.get('sibling').text());
assert.equal(str, doc.toString());
});

it('can be done by file', function() {
var doc = libxml.parseXmlFile(filename);
assert.equal('1.0', doc.version());
assert.equal('UTF-8', doc.encoding());
assert.equal('root', doc.root().name());
assert.equal('child', doc.get('child').name());
assert.equal('grandchild', doc.get('child').get('grandchild').name());
assert.equal('with love', doc.get('child/grandchild').text());
assert.equal('sibling', doc.get('sibling').name());
assert.equal('with content!', doc.get('sibling').text());
});
});

describe('A fatal parse error when parsing an XML file', function() {
var filename = path.dirname(__filename)+'/fixtures/errors/comment.xml';
var err = null;

it('will throw an exception', function() {
try {
libxml.parseXmlFile(filename);
} catch(e) { err = e; }

var errorControl = {
domain: 1,
code: 4,
message: "Start tag expected, '<' not found\n",
level: 3,
file: filename,
line: 5,
str1: null,
str2: null,
str3: null,
int1: null,
column: 10
};
assert.deepEqual(errorControl, err);
});
});

describe('A recoverable parse error when parsing an XML file', function() {
var filename = path.dirname(__filename)+'/fixtures/warnings/ent9.xml';

it('will attach the errors to the document', function() {
var doc = libxml.parseXmlFile(filename);
var err = {
domain: 3,
code: 201,
message: "Namespace prefix prefix on node is not defined\n",
level: 2,
file: null,
line: 1,
str1: "prefix",
str2: "node",
str3: null,
int1: null,
column: 13
};
assert.equal(1, doc.errors().length);
assert.equal(err.code, doc.errors()[0].code);
});
});

describe('A fatal parse error when parsing an XML string', function() {
Expand Down Expand Up @@ -118,16 +57,13 @@ describe('A recoverable parse error when parsing an XML string', function() {
code: 201,
message: "Namespace prefix prefix on node is not defined\n",
level: 2,
file: null,
line: 1,
str1: "prefix",
str2: "node",
str3: null,
int1: null,
column: 13
};
assert.equal(1, doc.errors().length);
assert.equal(err.code, doc.errors()[0].code);
assert.equal(1, doc.errors.length);
assert.deepEqual(err, doc.errors.shift());
});
});

Expand Down
90 changes: 22 additions & 68 deletions src/html_parser.cc
View file
Expand Up @@ -6,83 +6,37 @@

namespace libxmljs {

inline v8::Handle<v8::Value>
BuildDoc(xmlDoc *doc, v8::Handle<v8::Array> errors) {
v8::HandleScope scope;
if (doc == NULL) {
xmlFree(doc);
xmlError *error = xmlGetLastError();
if (error) {
return v8::ThrowException(XmlSyntaxError::BuildSyntaxError(error));
} else {
return v8::ThrowException(v8::Exception::Error(
v8::String::New("Could not parse XML string")));
}

return v8::Null();
}

v8::Handle<v8::Object> jsDoc =
LibXmlObj::GetMaybeBuild<HtmlDocument, xmlDoc>(doc);
HtmlDocument *document = LibXmlObj::Unwrap<HtmlDocument>(jsDoc);
document->errors = v8::Persistent<v8::Array>::New(errors);
return scope.Close(jsDoc);
}

v8::Handle<v8::Value>
ParseHtmlString(const v8::Arguments& args) {
v8::HandleScope scope;

if (!args[0]->IsString())
return v8::ThrowException(v8::Exception::Error(
v8::String::New("Must supply parseHtmlString with a string")));
v8::HandleScope scope;

v8::Persistent<v8::Array> errors = v8::Persistent<v8::Array>::New(
v8::Array::New());
xmlResetLastError();
xmlSetStructuredErrorFunc(reinterpret_cast<void *>(*errors),
XmlSyntaxError::PushToArray);
if (!args[0]->IsString())
return v8::ThrowException(v8::Exception::Error(
v8::String::New("Must supply parseHtmlString with a string")));

v8::String::Utf8Value str(args[0]->ToString());
htmlDocPtr doc = htmlReadMemory(*str, str.length(), NULL, NULL, 0);
v8::Local<v8::Array> errors = v8::Array::New();
xmlResetLastError();
xmlSetStructuredErrorFunc(reinterpret_cast<void *>(*errors),
XmlSyntaxError::PushToArray);

xmlSetStructuredErrorFunc(NULL, NULL);
v8::String::Utf8Value str(args[0]->ToString());
htmlDocPtr doc = htmlReadMemory(*str, str.length(), NULL, NULL, 0);

return scope.Close(BuildDoc(doc, errors));
}
xmlSetStructuredErrorFunc(NULL, NULL);

v8::Handle<v8::Value>
ParseHtmlFile(const v8::Arguments& args) {
v8::HandleScope scope;

if (!args[0]->IsString())
return v8::ThrowException(v8::Exception::Error(
v8::String::New("Must supply parseHtmlFile with a filename")));

v8::Persistent<v8::Array> errors = v8::Persistent<v8::Array>::New(
v8::Array::New());
xmlResetLastError();
xmlSetStructuredErrorFunc(reinterpret_cast<void *>(*errors),
XmlSyntaxError::PushToArray);

v8::String::Utf8Value str(args[0]->ToString());
htmlDocPtr doc = htmlReadFile(*str, NULL, 0);
if (!doc) {
xmlError* error = xmlGetLastError();
if (error) {
return v8::ThrowException(XmlSyntaxError::BuildSyntaxError(error));
}
return v8::ThrowException(v8::Exception::Error(
v8::String::New("Could not parse XML string")));
}

xmlSetStructuredErrorFunc(NULL, NULL);
v8::Handle<v8::Object> built_doc = LibXmlObj::GetMaybeBuild<HtmlDocument, xmlDoc>(doc);
built_doc->Set(v8::String::New("errors"), errors);

return scope.Close(BuildDoc(doc, errors));
return scope.Close(built_doc);
}

void
HtmlParser::Initialize(v8::Handle<v8::Object> target) {
v8::HandleScope scope;

LIBXMLJS_SET_METHOD(target,
"parseHtmlString",
ParseHtmlString);

LIBXMLJS_SET_METHOD(target,
"parseHtmlFile",
ParseHtmlFile);
}
} // namespace libxmljs
7 changes: 2 additions & 5 deletions src/html_parser.h
View file
Expand Up @@ -6,11 +6,8 @@

namespace libxmljs {

class HtmlParser : public LibXmlObj {
public:

static void Initialize(v8::Handle<v8::Object> target);
};
v8::Handle<v8::Value>
ParseHtmlString(const v8::Arguments& args);

} // namespace libxmljs

Expand Down
14 changes: 9 additions & 5 deletions src/libxmljs.cc
View file
Expand Up @@ -3,11 +3,11 @@
#include <v8.h>

#include "libxmljs.h"
#include "xml_syntax_error.h"
#include "xml_document.h"
#include "xml_node.h"
#include "xml_parser.h"
#include "xml_sax_parser.h"
#include "html_parser.h"
#include "xml_parser.h"

namespace libxmljs {

Expand Down Expand Up @@ -106,10 +106,14 @@ extern "C" void
init(v8::Handle<v8::Object> target) {
v8::HandleScope scope;

XmlSyntaxError::Initialize(target);
XmlDocument::Initialize(target);
XmlParser::Initialize(target);
HtmlParser::Initialize(target);
XmlSaxParser::Initialize(target);

NODE_SET_METHOD(target, "parseHtmlString",
libxmljs::ParseHtmlString);

NODE_SET_METHOD(target, "parseXmlString",
libxmljs::ParseXmlString);

target->Set(v8::String::NewSymbol("version"),
v8::String::New(LIBXMLJS_VERSION));
Expand Down
1 change: 0 additions & 1 deletion src/object_wrap.h
View file
Expand Up @@ -32,7 +32,6 @@ class LibXmlObj : public node::ObjectWrap {

return scope.Close(obj->handle_);
}

};

} // namespace libxmljs
Expand Down
13 changes: 0 additions & 13 deletions src/xml_document.cc
View file
Expand Up @@ -13,15 +13,6 @@ XmlDocument::Doc(const v8::Arguments& args) {
return scope.Close(args.This());
}

v8::Handle<v8::Value>
XmlDocument::Errors(const v8::Arguments& args) {
v8::HandleScope scope;
XmlDocument *document = LibXmlObj::Unwrap<XmlDocument>(args.This());
assert(document);

return scope.Close(document->errors);
}

v8::Handle<v8::Value>
XmlDocument::Encoding(const v8::Arguments& args) {
v8::HandleScope scope;
Expand Down Expand Up @@ -244,10 +235,6 @@ XmlDocument::Initialize(v8::Handle<v8::Object> target) {
"document",
XmlDocument::Doc);

LXJS_SET_PROTO_METHOD(constructor_template,
"errors",
XmlDocument::Errors);

LXJS_SET_PROTO_METHOD(constructor_template,
"toString",
XmlDocument::ToString);
Expand Down
1 change: 0 additions & 1 deletion src/xml_document.h
View file
Expand Up @@ -12,7 +12,6 @@ class XmlDocument : public LibXmlObj {
virtual ~XmlDocument();

xmlDoc* xml_obj;
v8::Persistent<v8::Array> errors;
explicit XmlDocument(xmlDoc* doc) : xml_obj(doc) {}
static void Initialize(v8::Handle<v8::Object> target);
static v8::Persistent<v8::FunctionTemplate> constructor_template;
Expand Down

0 comments on commit 251b18a

Please sign in to comment.