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

Handle 'status: fail' responses from PhantomJS land instead of returning minimal HTML (i.e. "blank page") #473

Merged
merged 1 commit into from Apr 13, 2014

Conversation

Projects
None yet
5 participants
@dholdren
Contributor

dholdren commented Apr 3, 2014

Problem:

When there is a DNS issue, or the server is down, Poltergeist will return a body like this:

<html><head></head><body></body></html>

In your capybara tests then, you will get Unable to find field "foo" (Capybara::ElementNotFound) when the actual problem is
a) your host name is unusable (i.e. what you set for Capybara.default_host or Capybara.app_host is not in /etc/hosts)
b) the server died somehow

Solution:

Detect a PhantomJS "fail" status.

When PhantomJS can't reach a server, it returns an actionable "status" value, and, oddly, a minimal HTML structure for page.body. Poltergeist should detect the status and raise an Error.

var page = require('webpage').create();

var url = "http://nope";

page.onResourceReceived = function(resource) {
    console.log('Url: ' + resource.url);
    console.log('Code: ' + resource.status);
}

page.open(url, function(status) {
  console.log('Status: ' + JSON.stringify(status));
  console.log('Page: '+JSON.stringify(page));
  phantom.exit();
});
Url: http://nope/
Code: null
Status: "fail"
Page: {
  "objectName": "WebPage",
  "title": "",
  "frameTitle": "",
  "content": "<html><head></head><body></body></html>",
  "frameContent": "<html><head></head><body></body></html>",
  "url": "about:blank",
  "frameUrl": "about:blank",
  "loading": false,
  "loadingProgress": 100,
  "canGoBack": false,
  "canGoForward": false,
  "plainText": "",
  "framePlainText": "",
  "libraryPath": "/Users/dholdren/tmp",
  "offlineStoragePath": "/Users/dholdren/Library/Application Support/Ofi Labs/PhantomJS",
  "offlineStorageQuota": 5242880,
  "viewportSize": {
    "height": 300,
    "width": 400
  },
  "paperSize": {},
  "clipRect": {
    "height": 0,
    "left": 0,
    "top": 0,
    "width": 0
  },
  "scrollPosition": {
    "left": 0,
    "top": 0
  },
  "navigationLocked": false,
  "customHeaders": {},
  "zoomFactor": 1,
  "cookies": [],
  "windowName": "",
  "pages": [],
  "pagesWindowName": [],
  "ownsPages": true,
  "framesName": [],
  "frameName": "",
  "framesCount": 0,
  "focusedFrameName": ""
}

For a comparison of a "fail" vs. a "success" in PhantomJS:
https://gist.github.com/dholdren/9942454

Show outdated Hide outdated lib/capybara/poltergeist/client/compiled/browser.js
@@ -85,6 +94,8 @@ Poltergeist.Browser = (function() {
this.page.clearErrors();
if (errors.length > 0 && this.js_errors) {
return this.owner.sendError(new Poltergeist.JavascriptError(errors));
} else if (response['status'] === 'fail') {

This comment has been minimized.

@yaauie

yaauie Apr 3, 2014

Member

Travis reports that this line is failing all over the place:

TypeError: 'undefined' is not an object (evaluating 'response['status']')
    at /home/travis/build/teampoltergeist/poltergeist/lib/capybara/poltergeist/client/compiled/browser.js:97
@yaauie

yaauie Apr 3, 2014

Member

Travis reports that this line is failing all over the place:

TypeError: 'undefined' is not an object (evaluating 'response['status']')
    at /home/travis/build/teampoltergeist/poltergeist/lib/capybara/poltergeist/client/compiled/browser.js:97
Show outdated Hide outdated lib/capybara/poltergeist/client/browser.coffee
@@ -66,6 +66,8 @@ class Poltergeist.Browser
if errors.length > 0 && @js_errors
@owner.sendError(new Poltergeist.JavascriptError(errors))
else if response['status'] == 'fail'

This comment has been minimized.

@yaauie

yaauie Apr 3, 2014

Member

So it looks like this is the cause of the build failures.

What should we do here if response is undefined? Is it a StatusFailError, or something else entirely?

@yaauie

yaauie Apr 3, 2014

Member

So it looks like this is the cause of the build failures.

What should we do here if response is undefined? Is it a StatusFailError, or something else entirely?

This comment has been minimized.

@dholdren

dholdren Apr 3, 2014

Contributor

ehh. rookie mistake, was running targeted specs, forgot to run them all at the end. will fix

@dholdren

dholdren Apr 3, 2014

Contributor

ehh. rookie mistake, was running targeted specs, forgot to run them all at the end. will fix

This comment has been minimized.

@yaauie

yaauie Apr 3, 2014

Member

quite possibly the phantomjs version?

@yaauie

yaauie Apr 3, 2014

Member

quite possibly the phantomjs version?

@dholdren

This comment has been minimized.

Show comment
Hide comment
@dholdren

dholdren Apr 3, 2014

Contributor

@yaauie I fixed most of the failing specs, there are some remaining ones related to basic-auth failure cases - Looking into those now.

Contributor

dholdren commented Apr 3, 2014

@yaauie I fixed most of the failing specs, there are some remaining ones related to basic-auth failure cases - Looking into those now.

Show outdated Hide outdated lib/capybara/poltergeist/client/browser.coffee
@@ -66,6 +66,8 @@ class Poltergeist.Browser
if errors.length > 0 && @js_errors
@owner.sendError(new Poltergeist.JavascriptError(errors))
else if @page['_statusCode'] == null && response? && response['status'] == 'fail'

This comment has been minimized.

@yaauie

yaauie Apr 3, 2014

Member

Disclaimer: I'm not a coffeescript person so I could be completely wrong.

It looks like you're reaching into the compiled javascript to get that instance variable with @page['_statusCode'], which gets translated to this.page['_statusCode'] in javascript; this works, but is fragile because you're both reaching into an object's private api and depending on technicalities in the coffeescript->javascript translation.

It can be avoided in pure coffeescript by using the @page.statusCode method, which exposes that instance variable for public reading.

@yaauie

yaauie Apr 3, 2014

Member

Disclaimer: I'm not a coffeescript person so I could be completely wrong.

It looks like you're reaching into the compiled javascript to get that instance variable with @page['_statusCode'], which gets translated to this.page['_statusCode'] in javascript; this works, but is fragile because you're both reaching into an object's private api and depending on technicalities in the coffeescript->javascript translation.

It can be avoided in pure coffeescript by using the @page.statusCode method, which exposes that instance variable for public reading.

This comment has been minimized.

@dholdren

dholdren Apr 4, 2014

Contributor

yep. Fixed. I was looking at some raw debug output when I wrote that. thanks

@dholdren

dholdren Apr 4, 2014

Contributor

yep. Fixed. I was looking at some raw debug output when I wrote that. thanks

@yaauie

This comment has been minimized.

Show comment
Hide comment
@yaauie

yaauie Apr 4, 2014

Member

LGTM 👍. If you know how, can you squash it into one commit and re-push?

I'd like to leave this pull-request open for a couple days until I (or another committer)
get a chance to try it out. Thanks a bunch!

Member

yaauie commented Apr 4, 2014

LGTM 👍. If you know how, can you squash it into one commit and re-push?

I'd like to leave this pull-request open for a couple days until I (or another committer)
get a chance to try it out. Thanks a bunch!

@thedelchop

This comment has been minimized.

Show comment
Hide comment
@thedelchop

thedelchop Apr 11, 2014

Contributor

LGTM as well 👍 I'm new to the poltergeist team, but I'm in agreement with @yaauie lets merge this one in. Thanks!

Contributor

thedelchop commented Apr 11, 2014

LGTM as well 👍 I'm new to the poltergeist team, but I'm in agreement with @yaauie lets merge this one in. Thanks!

@thedelchop

This comment has been minimized.

Show comment
Hide comment
@thedelchop

thedelchop Apr 12, 2014

Contributor

@dholdren Can you just rebase this against master and I'll accept it as soon as you update it.

Contributor

thedelchop commented Apr 12, 2014

@dholdren Can you just rebase this against master and I'll accept it as soon as you update it.

Handle 'status:fail' responses from PhantomJS land
When a server is not running at the specified host:port or
if there is a DNS failure, PhantomJS will respond with
status=fail, no response code, and a minimal HTML document.
We should not return that document, but throw an exception instead.
@dholdren

This comment has been minimized.

Show comment
Hide comment
@dholdren

dholdren Apr 13, 2014

Contributor

@thedelchop I rebased against teampoltergeist master yesterday in case you missed it

Contributor

dholdren commented Apr 13, 2014

@thedelchop I rebased against teampoltergeist master yesterday in case you missed it

@thedelchop

This comment has been minimized.

Show comment
Hide comment
@thedelchop

thedelchop Apr 13, 2014

Contributor

Yeah, I see that, thanks for the work.

Contributor

thedelchop commented Apr 13, 2014

Yeah, I see that, thanks for the work.

thedelchop added a commit that referenced this pull request Apr 13, 2014

Merge pull request #473 from dholdren/master
Handle 'status: fail' responses from PhantomJS land instead of returning minimal HTML (i.e. "blank page")

@thedelchop thedelchop merged commit 41380ee into teampoltergeist:master Apr 13, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@route

This comment has been minimized.

Show comment
Hide comment
@route

route May 26, 2014

Member

The code looks good to me, but I have to add a few words here. Is this behavior in consistency with Selenium for example?

Member

route commented May 26, 2014

The code looks good to me, but I have to add a few words here. Is this behavior in consistency with Selenium for example?

@dholdren

This comment has been minimized.

Show comment
Hide comment
@dholdren

dholdren Jun 12, 2014

Contributor

@route It's consistent with the headless driver capybara-webkit:

capybara-webkit (1.1.0)

@javascript
Feature: Lets' go to the home page!
  We should go to the home page

  Scenario: Going to the home page should show "Home Page" # features/home_page.feature:5
    When I go to the home page                             # features/step_definitions/web_steps.rb:24
      Unable to load URL: http://foobar.what:60001/ because of error loading http://foobar.what:60001/: Unknown error (Capybara::Webkit::InvalidResponseError)
      ./features/step_definitions/web_steps.rb:25:in `/^(?:|I )go to (.+)$/'
      features/home_page.feature:6:in `When I go to the home page'
    Then I should see "Home Page"                          # features/step_definitions/web_steps.rb:161

Failing Scenarios:
cucumber features/home_page.feature:5 # Scenario: Going to the home page should show "Home Page"

1 scenario (1 failed)
2 steps (1 failed, 1 skipped)
0m0.783s

poltergeist (master)

@javascript
Feature: Lets' go to the home page!
  We should go to the home page

  Scenario: Going to the home page should show "Home Page" # features/home_page.feature:5
    When I go to the home page                             # features/step_definitions/web_steps.rb:24
      Request failed to reach server, check DNS and/or server status (Capybara::Poltergeist::StatusFailError)
      ./features/step_definitions/web_steps.rb:25:in `/^(?:|I )go to (.+)$/'
      features/home_page.feature:6:in `When I go to the home page'
    Then I should see "Home Page"                          # features/step_definitions/web_steps.rb:161

Failing Scenarios:
cucumber features/home_page.feature:5 # Scenario: Going to the home page should show "Home Page"

1 scenario (1 failed)
2 steps (1 failed, 1 skipped)
0m1.734s
Contributor

dholdren commented Jun 12, 2014

@route It's consistent with the headless driver capybara-webkit:

capybara-webkit (1.1.0)

@javascript
Feature: Lets' go to the home page!
  We should go to the home page

  Scenario: Going to the home page should show "Home Page" # features/home_page.feature:5
    When I go to the home page                             # features/step_definitions/web_steps.rb:24
      Unable to load URL: http://foobar.what:60001/ because of error loading http://foobar.what:60001/: Unknown error (Capybara::Webkit::InvalidResponseError)
      ./features/step_definitions/web_steps.rb:25:in `/^(?:|I )go to (.+)$/'
      features/home_page.feature:6:in `When I go to the home page'
    Then I should see "Home Page"                          # features/step_definitions/web_steps.rb:161

Failing Scenarios:
cucumber features/home_page.feature:5 # Scenario: Going to the home page should show "Home Page"

1 scenario (1 failed)
2 steps (1 failed, 1 skipped)
0m0.783s

poltergeist (master)

@javascript
Feature: Lets' go to the home page!
  We should go to the home page

  Scenario: Going to the home page should show "Home Page" # features/home_page.feature:5
    When I go to the home page                             # features/step_definitions/web_steps.rb:24
      Request failed to reach server, check DNS and/or server status (Capybara::Poltergeist::StatusFailError)
      ./features/step_definitions/web_steps.rb:25:in `/^(?:|I )go to (.+)$/'
      features/home_page.feature:6:in `When I go to the home page'
    Then I should see "Home Page"                          # features/step_definitions/web_steps.rb:161

Failing Scenarios:
cucumber features/home_page.feature:5 # Scenario: Going to the home page should show "Home Page"

1 scenario (1 failed)
2 steps (1 failed, 1 skipped)
0m1.734s
@jure

This comment has been minimized.

Show comment
Hide comment
@jure

jure Sep 1, 2014

Hmm, not yet sure if this is supposed to cover my scenario: running Rails/Poltergeist/Capybara/RSpec/Webmock and the app crashes because of an unstubbed request my app is trying to make:

2014-09-01 20:41:52.511 [INFO ] Completed 500 Internal Server Error in 6.5ms (pid:19103)

But from the Poltergeist site, all I get is:

{"name"=>"visit", "args"=>["http://127.0.0.1:50529/"]}
poltergeist [1409604937785] state default -> loading
poltergeist [1409604937850] state loading -> default
{"response"=>{"status"=>"success"}}

and the page.html is: <html><head></head><body></body></html>

Went digging a bit and the config/environments/test.rb show exception setting was false:

# Raise exceptions instead of rendering exception templates
config.action_dispatch.show_exceptions = false

After setting it to true, I get a detailed exception in my logs and as page.html, but Poltergeist still says {"response"=>{"status"=>"success"}}

Am I just walking on the edge a bit too much?

jure commented Sep 1, 2014

Hmm, not yet sure if this is supposed to cover my scenario: running Rails/Poltergeist/Capybara/RSpec/Webmock and the app crashes because of an unstubbed request my app is trying to make:

2014-09-01 20:41:52.511 [INFO ] Completed 500 Internal Server Error in 6.5ms (pid:19103)

But from the Poltergeist site, all I get is:

{"name"=>"visit", "args"=>["http://127.0.0.1:50529/"]}
poltergeist [1409604937785] state default -> loading
poltergeist [1409604937850] state loading -> default
{"response"=>{"status"=>"success"}}

and the page.html is: <html><head></head><body></body></html>

Went digging a bit and the config/environments/test.rb show exception setting was false:

# Raise exceptions instead of rendering exception templates
config.action_dispatch.show_exceptions = false

After setting it to true, I get a detailed exception in my logs and as page.html, but Poltergeist still says {"response"=>{"status"=>"success"}}

Am I just walking on the edge a bit too much?

@route

This comment has been minimized.

Show comment
Hide comment
@route

route Sep 1, 2014

Member

@jure What's the response status?

Member

route commented Sep 1, 2014

@jure What's the response status?

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