brenden / node-webshot

Easy website screenshots in Node.js
2.12k stars 285 forks source link

Add option 'errorIfStatusIsNot200'. #61

Closed amyboyd closed 10 years ago

brenden commented 10 years ago

Hi @amyboyd,

This is a good idea. However that implementation will cause webshot to return an error whenever any resource on the page fails because onResourceReceived runs for every request, not just when the page initially loads. I assume that's not the intended behavior... I think just looking at the status argument of _takeScreenshot is enough to determine whether the requested page has a status of 200 or not.

bhurlow commented 10 years ago

+1 for the general principle here. I'm using webshot in production where it has to deal with a lot of 401 so I'm getting back a lot of broken images or what have you

amyboyd commented 10 years ago

Hi Brenden,

This will only affect the target URL, not images (see if (response.url === site).

The status for _takeScreenshot is the string "fail" only if PhantomJS experiences an error, and is the string "success" otherwise.

brenden commented 10 years ago

Ah my mistake, you're right on both points. Should have looked more closely at your commit.

Thanks for adding this!