Browser logging implementation and Network Activity logging in HAR format support #275
Conversation
|
@detro have you any comment or feedback on my PR? |
|
Overall I think those features are needed and welcome. But some of this might be clashing with the PR #274. The coding is OK: you have respected the "implicit coding standard" :) The one issue I can see is that you have a lot of commits that are all related to the features you are adding: you will need to squash this stuff into one (or max 2/3). |
|
So, I'm going to merge this PR but I'll manually integrate the feature of #274 as I think it can still be very useful to optionally output Network Req/Res in the console, if the right Capability is set. |
|
I reviewed related PR about network logging. As I understand you plan that HAR loggin will not enabled by default, but can be enabled using capabilities. Am I right? |
|
I'm working on merging this but in latest PhantomJS/master the page.onError code seems to have issues. I'm investigating. |
There was a problem hiding this comment.
page.resources[error.id] object needs to be initialized
There was a problem hiding this comment.
I'm fixing it as part of the merge.
Curiosity: did you run the tests?
There was a problem hiding this comment.
After that fixes seems like no. My fault, sorry about this.
There was a problem hiding this comment.
Btw You need to add one line:
page.onResourceError = function(error) {
page.resources[error.id] || (page.resources[error.id] = {});
|
Merged (and added few fixed + test). Thanks! :) |
2 participants
No description provided.