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

Getting a stream event to fire when I don't think it should be? #94

Open
ralucas opened this issue Jan 9, 2015 · 2 comments
Open

Getting a stream event to fire when I don't think it should be? #94

ralucas opened this issue Jan 9, 2015 · 2 comments

Comments

@ralucas
Copy link

ralucas commented Jan 9, 2015

Hi, I'm getting a stream event to fire when I don't think it should be:

app.post('/renderUrl', urlEncodedParser, function(req, res) {
  var url = req.body.url;
  var filepath = '/path/to/file.png';

  var options = {
    siteType: 'url',
    streamType: 'png',
    errorIfStatusIsNot200: true
  };

  webshot(url, options, function(err, renderStream) {
    if (err) {
      if (/Status must be 200/.test(err.message)) {
        res.send('urlFailed'); //this is firing on a 404
      } else {
        res.send(err);
      }

    } else {
      var file = fs.createWriteStream(path.join(__dirname, filepath), {encoding: 'binary'});

      renderStream.on('data', function(data) {
        file.write(data.toString('base64'), 'binary');
      });

      renderStream.on('end', function(dt) {
        console.log('in the renderstream end'); //This console.log is firing on a 404
        fs.readFile(path.join(__dirname, filepath), function(err, data) {
          if (err) {
            res.send(err);
          } else {
            //these events are firing on a 404
            res.send(data);
          }
        });
      });
    }
  });
});

Am I thinking about this incorrectly or something bad here in the code?

Thank you for any help.

Cheers!

Richard

@troyfendall
Copy link

I had this problem as well. Unfortunately I couldn't find a good solution that allowed me to use a stream and have webshot throw an error for a non-200 result. I found that it does throw an error if you write the webshot to a file rather than using a stream, but if you ultimately want to work with a stream, writing it to a temp file then opening a read stream to that file is suboptimal to say the least. It would be nice if webshot could recognize a non-200 error and throw the error itself, or at least take an http stream as an alternate input since other http libraries throw an error on 4** and 5** results.

My solution, since I'm in control of the webpage being rendered, and I know that it's fast and little content, is that I wrap the call to webshot in an up-front http request, which will throw the error I need before I initiate webshot.

Not sure if that helps, but I figured I'd +1 the issue.

@msurdi
Copy link

msurdi commented Jan 20, 2016

Looking at this line, seems like if the subprocess outputs to stdout an error, the only possibility considered for it is to be a js error, when the truth seems to be that for non 200 errors, the process also writes to stderr.

To me, it looks like that code should read something more like:

phantomProc.stderr.on('data', function(data) {
  if (options.errorIfJSException || options.errorIfStatusIsNot200) {
    s.emit('error', ''+data);
  }
});

for now, my workaround has been to enable errorIfJSException and do in my own code something like:

let stream = webshot(.........)
stream.on("data", /*happy path*/)
stream.on("error", /*error path*/)

keeping in mind that the happy path will be always called, but in the case of an error the error path will have been called before so you can set a flag or something.

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

No branches or pull requests

3 participants