cyrus-and / chrome-har-capturer

Capture HAR files from a Chrome instance
MIT License
530 stars 90 forks source link

Hanging on events without a handler #61

Closed jdcc closed 6 years ago

jdcc commented 6 years ago

There's no else clause on this condition, so if I pass in an event that doesn't have a handler, my promise sits unfulfilled forever.

I think the correct behavior is probably just to fulfill if there's no handler, but not sure how you want to handle it.

cyrus-and commented 6 years ago

How should I reproduce the hanging? Show me the code.

jdcc commented 6 years ago

OK, it's something more complicated than I described. Simply creating an unhandled event doesn't seem to do it.

I can reproduce similar behavior with this (large) events file and this code:

const fs = require('fs');
const chromeHar = require('chrome-har-capturer');
const events = JSON.parse(fs.readFileSync('events.json'));
chromeHar.fromLog('http://nyt.com', events, { content: true }).then(har => {
  console.log('done');
});

Running this, I'd expect to see "done" logged to stdout. Instead, I get nothing. If I fulfill the promises of unhandled events, I do indeed see "done" on stdout.

In testing this, I noticed that if I pass an events file without the Network.getResponseBody events but still pass { content: true }, I also do not get "done" on stdout. If I remove the { content: true } option, things happen as expected.

cyrus-and commented 6 years ago

If I fulfill the promises of unhandled events, I do indeed see "done" on stdout.

But you'll probably get an invalid HAR file.

https://github.com/cyrus-and/chrome-har-capturer/blob/35741696e20f264446c81948cdc37d1a82ceda95/lib/stats.js#L19-L21

If there is no handler nothing is to be done for that event. That handler is not an user-supplied one, for this reason you're unable to reproduce it by creating an unhandled event.

In testing this, I noticed that if I pass an events file without the Network.getResponseBody events but still pass { content: true }, I also do not get "done" on stdout. If I remove the { content: true } option, things happen as expected.

This is the expected behavior as the log file is scanned for Network.getResponseBody but there are none, so there's nothing to generate. I should probably return an error in this case...

Running this, I'd expect to see "done" logged to stdout. Instead, I get nothing.

But if you set content: false if completes. This is most likely because your log is incomplete, meaning that it misses some Network.getResponseBody events.

cyrus-and commented 6 years ago

I should probably return an error in this case...

Done in 7fa3c57c3dadb996cb1a3ef80da0402b73134284.

cyrus-and commented 6 years ago

This is most likely because your log is incomplete, meaning that it misses some Network.getResponseBody events.

FTI you can figure this out like this:

$ jq '.[]|select(.method=="Network.loadingFinished")|.params.requestId' gistfile1.txt >Network.loadingFinished.ids
$ jq '.[]|select(.method=="Network.getResponseBody")|.params.requestId' gistfile1.txt >Network.getResponseBody.ids
$ diff -y <(sort Network.loadingFinished.ids) <(sort Network.getResponseBody.ids) | less

You'll see that there are some entries missing in the right column (the Network.getResponseBody.ids file).

jdcc commented 6 years ago

All makes sense, thank you. Also, thanks for mentioning jq - that's new to me, and it's pretty surprising I haven't stumbled across it before.

cyrus-and commented 6 years ago

You're welcome! jq is great and very powerful but I can't wrap my head around its syntax. :) Also worth to mention: gron.