Skip to content

Commit

Permalink
Clean up internal loops in console and pageerror event handlers
Browse files Browse the repository at this point in the history
Both the `console` and `pageerror` events are only passed a single
argument by Puppeteer. For the `console` the loop made it seem like
the event handler args are related to the `console.log()` args, but
this wasn't the case since the single "msg" object represents the call
overall, not one argument.

Also update the documentation to not document a `stackTrace` argument
as this did not actually exist. When `pageerror` is given an Error
object, as it usually is, it may have a trace property, however, but
there seems to be no such argument.

The good thing is that when Puppeteer emits `pageerror`, it already
serialised and re-created the Error object within Node.js, thus it
can be cleanly used and stringified in a useful manner by other code.

(Unlike when e.g. an Error object is passed to `console.error()`,
because "console" arguments are merely JSHandle references that have
to be asynchronously evaluated back via the browser to format as a
string. This would be nice to improve..., but that's for another
time.)

This commit should be a no-op.
  • Loading branch information
Krinkle committed Mar 12, 2022
1 parent 024838b commit f50ed47
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 16 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ In addition to QUnit callback-named events, the following events are emitted by
* `qunit.spawn` `(url)`: when [Chrome][] is spawned for a test
* `qunit.fail.load` `(url)`: when [Chrome][] could not open the given url
* `qunit.fail.timeout`: when a QUnit test times out, usually due to a missing `QUnit.start()` call
* `qunit.error.onError` `(message, stackTrace)`: when a JavaScript execution error occurs
* `qunit.error.onError` `(err)`: when a JavaScript execution error occurs

You may listen for these events like so:

Expand Down
2 changes: 1 addition & 1 deletion docs/qunit-examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ In addition to QUnit callback-named events, the following events are emitted by
* `qunit.spawn` `(url)`: when [Chrome][] is spawned for a test
* `qunit.fail.load` `(url)`: when [Chrome][] could not open the given url
* `qunit.fail.timeout`: when a QUnit test times out, usually due to a missing `QUnit.start()` call
* `qunit.error.onError` `(message, stackTrace)`: when a JavaScript execution error occurs
* `qunit.error.onError` `(err)`: when a JavaScript execution error occurs

You may listen for these events like so:

Expand Down
39 changes: 25 additions & 14 deletions tasks/qunit.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ module.exports = function(grunt) {
status.failed += 1;
});

eventBus.on('error.onError', function (msg, stackTrace) {
grunt.event.emit('qunit.error.onError', msg, stackTrace);
eventBus.on('error.onError', function (msg) {
grunt.event.emit('qunit.error.onError', msg);
});

grunt.registerMultiTask('qunit', 'Run QUnit unit tests in a headless Chrome instance.', function() {
Expand Down Expand Up @@ -378,26 +378,37 @@ module.exports = function(grunt) {
.then(function() {
// Pass through the console logs if instructed
if (options.console) {
page.on('console', function() {
var args = [].slice.apply(arguments);
page.on('console', function(msg) {
// The `msg` is a puppeteer.ConsoleMessage, which represents the console call
// including multple arguments passed to it.
//
// msg.text() formats the arguments into a naive newline-joined string, and
// includes error objects as a useless "JSHandle@error".
//
// msg.args() returns a JSHandle object for each argument, but all its
// evaluation features happen asynchronously via the browser, and in this
// event handler we can't await those easily as the grunt output will have
// moved on to other tests. If we want to print these, we'd have to refactor
// this so pEachSeries() below is aware of async code here. For now, we just
// let the useless "JSHandle" through and rely on developers to stringify any
// useful information ahead of time, e.g. `console.warn(String(err))`.
//
// Ref https://pptr.dev/#?product=Puppeteer&version=v5.0.0&show=api-class-consolemessage
var colors = {
'error': 'red',
'warning': 'yellow'
};
for (var i = 0; i < args.length; ++i) {
var txt = args[i].text();
var color = colors[args[i].type()];
grunt.log.writeln(color ? txt[color] : txt);
}
var txt = msg.text();
var color = colors[msg.type()];
grunt.log.writeln(color ? txt[color] : txt);
// grunt.log.writeln(`${msg.location().url}:${msg.location().lineNumber}`.gray); // debug
});
}

// Surface uncaught exceptions
page.on('pageerror', function() {
var args = [].slice.apply(arguments);
for (var i = 0; i < args.length; i++) {
eventBus.emit('error.onError', args[i]);
}
// Ref https://pptr.dev/#?product=Puppeteer&version=v5.0.0&show=api-event-pageerror
page.on('pageerror', function(err) {
eventBus.emit('error.onError', err);
});

// Whenever a page is loaded with a new document, before scripts execute, inject the bridge file.
Expand Down

0 comments on commit f50ed47

Please sign in to comment.