Skip to content
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.

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

Level 1 Core DOM Exception: Hierarchy_Request_Err #290

Closed
ghost opened this issue Sep 1, 2011 · 16 comments
Closed

Level 1 Core DOM Exception: Hierarchy_Request_Err #290

ghost opened this issue Sep 1, 2011 · 16 comments

Comments

@ghost
Copy link

ghost commented Sep 1, 2011

Reading this url: http://www.toshibadirect.com/td/b2c/laptops.to , I consistently get this error:

../node_modules/jsdom/lib/jsdom/level1/core.js:1275
throw new core.DOMException(HIERARCHY_REQUEST_ERR);
^
Error: Hierarchy request error
at Object.appendChild (/Users/funk/node_modules/jsdom/lib/jsdom/level1/core.js:1275:13)
at setChild (/Users/funk/node_modules/jsdom/lib/jsdom/browser/htmltodom.js:167:17)
at HtmlToDom.appendHtmlToElement (/Users/funk/node_modules/jsdom/lib/jsdom/browser/htmltodom.js:77:9)
at Object.innerHTML (/Users/funk/node_modules/jsdom/lib/jsdom/browser/index.js:384:27)
at Object.write (/Users/funk/node_modules/jsdom/lib/jsdom/level2/html.js:391:22)
at Object.jsdom (/Users/funk/node_modules/jsdom/lib/jsdom.js:53:9)
at Object.html (/Users/funk/node_modules/jsdom/lib/jsdom.js:91:18)
at /Users/funk/node_modules/jsdom/lib/jsdom.js:206:26
at Request.callback (/Users/funk/node_modules/jsdom/lib/jsdom.js:289:17)
at Request. (/Users/funk/node_modules/jsdom/node_modules/request/main.js:314:21)

The url is read from a standard jsdom idiom:

jsdom.env(url, [ 'http://code.jquery.com/jquery-1.6.2.min.js' ], function (error, window) { ... }.

Interestingly, I've wrapped the jsdom.env call in a try/catch block. Yet the exception is not trapped. Warning: I'm a javascript novice. But in java, a try/catch(throwable) would definitely catch the error.

@tmpvar
Copy link
Member

tmpvar commented Sep 1, 2011

the try/catch needs to happen inside of jsdom in a similar way that #279 works

@ghost
Copy link
Author

ghost commented Sep 1, 2011

interesting. Thanks for the javascript lesson. That said, the exception is still not caught with this jsdom idiom:

    jsdom.env(url, [ 'http://code.jquery.com/jquery-1.6.2.min.js' ], function (error, window) {
        try {
            if (error) {
                // handle error
            } else {
               // do work
            }
        } catch(exception) {
            // handle exception
        }
    });

Perhaps, I missing something in the translation. Naturally, as expected, the originally reported error is still thrown. I was just hoping to check the error or catch the exception.

On Sep 1, 2011, at 2:16 PM, tmpvar wrote:

the try/catch needs to happen inside of jsdom in a similar way that #279 works

Reply to this email directly or view it on GitHub:
#290 (comment)

@tmpvar
Copy link
Member

tmpvar commented Sep 1, 2011

at that point the exception has already been thrown

@ghost
Copy link
Author

ghost commented Sep 1, 2011

That's what I thought, which is why I originally wrapped the try/catch around the jsdom.env block. Either way the exception is not caught. I've returned to my original try/catch around the jsdom.env block. Frankly, I don't get why the error is not trapped..

Sorry, didn't mean to detract from the original dom exception.

On Sep 1, 2011, at 2:43 PM, tmpvar wrote:

at that point the exception has already been thrown

Reply to this email directly or view it on GitHub:
#290 (comment)

@tmpvar
Copy link
Member

tmpvar commented Sep 1, 2011

The main reason why you can't catch on the outside of the jsdom.env block is because the exception is happening in another tick of the event loop.

The fix would be to find the line the exception is being thrown on, catch it, and report it so that it is returned in the errors array in the callback. If you are feeling up to it, I can help you through the process

@ghost
Copy link
Author

ghost commented Sep 1, 2011

I'm getting ready to leave the office. But let me research your 'another tick of the event loop' comment. And if I need help, I'll fire off a message. Thanks!

@tmpvar
Copy link
Member

tmpvar commented Sep 1, 2011

@ghost
Copy link
Author

ghost commented Sep 1, 2011

Interesting. Implemented. Thanks!

On Sep 1, 2011, at 2:55 PM, tmpvar wrote:

see: http://nodejs.org/docs/v0.3.1/api/process.html#event_uncaughtException_

Reply to this email directly or view it on GitHub:
#290 (comment)

@tmpvar
Copy link
Member

tmpvar commented Sep 1, 2011

haha, that doesn't solve the underlying issue though...

@ghost
Copy link
Author

ghost commented Sep 1, 2011

Agreed. But a win is a win, as they say in sports.;)

On Sep 1, 2011, at 4:26 PM, tmpvar wrote:

haha, that doesn't solve the underlying issue though...

Reply to this email directly or view it on GitHub:
#290 (comment)

@nauhygon
Copy link

We are seeing the same issue in Zombie (which depends on jsdom). The issue here appears to be related to the extra contents after the closing </html>. For a real browser, it would happily append them to <body>. The fix #387 avoids the exception, but prevents jsdom from behaving like a real browser. So maybe we should add this change in addition to #387?

@lbdremy
Copy link
Contributor

lbdremy commented Jan 24, 2012

Hi @nauhygon,

I added your change in the code, it works fine. I tested with this:

issue_290_behave_like_a_browser_if_issue_319 : function(test){
   jsdom.env({
      html: '<!DOCTYPE html><html><head><title>Title</title></head><body>My body</body></html><div id="nice">Yes</div>',
      done : function(err,window){
          test.equal(window.document.getElementById('nice').innerHTML,'Yes');
          test.done();
      }
   });
  }

But it has for effect to make this test fail because now no exception is thrown instead the new html element created is appended in the body tag.

appendChild_to_document_with_existing_documentElement: function(test) {
    test.expect(2);
    t = function(){
      try {
        var doc = jsdom.jsdom();
        doc.appendChild(doc.createElement('html'));
      } catch (e) {
        test.equal(e.code, 3, 'Should throw HIERARCHY_ERR');
        code = e.code;
        throw(e);
      }
    };
    test.throws(t);
    test.done();
  },

What do you think?

@nauhygon
Copy link

Thanks @lbdremy. The failure is expected from the change. I think it's really a matter of whether we want jsdom to behave like a regular browser in this case, i.e. more tolerant of the type of ill-formed html content. Or, if we want to retain the old behavior, we could introduce a new option. It would be really nice to hear what the jsdom gurus (e.g. @tmpvar) have to say about this.

@joshsmith
Copy link

Would love to have someone check out lbdremy's fix. This breaks on yahoo.com, which is pretty bad.

@tmpvar
Copy link
Member

tmpvar commented Feb 17, 2012

@nauhygon so the idea behind jsdom is to be compliant with the spec. However, browser makers have tendency to go off the rails. So we typically try to emulate the best case browser behavior when it is needed.

In this case I think it seems optimal to handle this in the way @lbdremy has presented. I'll be trying out the patch shortly, thanks!

@ehartford
Copy link

lbdremy's patch works great for me. I have patched my zombie with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants