console.error is not a function #4467

Closed
jasnell opened this Issue Dec 29, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@jasnell
Member

jasnell commented Dec 29, 2015

Using 4.2.4 on OSX (not sure if this appears anywhere else yet).

Script: test.js

'use strict';
const EventEmitter = require('events');

EventEmitter.defaultMaxListeners = 1;

class MyEmitter extends EventEmitter {}

const myEmitter = new MyEmitter();
myEmitter.on('event', ()=>{});
myEmitter.on('event', ()=>{});

myEmitter.emit('event');

Output:

bash-3.2$ node ~/tmp/test.js
events.js:235
        console.error('(node) warning: possible EventEmitter memory ' +
                ^

TypeError: console.error is not a function
    at process.addListener (events.js:235:17)
    at process.stderr (node.js:652:17)
    at console.js:92:53
    at NativeModule.compile (node.js:954:5)
    at Function.NativeModule.require (node.js:902:18)
    at node.js:200:27
    at MyEmitter.addListener (events.js:235:9)
    at Object.<anonymous> (/Users/james/tmp/test.js:10:11)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
bash-3.2$ 
@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Dec 29, 2015

Member

Same error does not occur using the repl:

bash-3.2$ node
> const EventEmitter = require('events');
undefined
> EventEmitter.defaultMaxListeners = 1
1
> class MyEmitter extends EventEmitter {}
[Function: MyEmitter]
> const myEmitter = new MyEmitter();
undefined
> myEmitter.on('event', ()=>{});
MyEmitter {
  domain: 
   Domain {
     domain: null,
     _events: { error: [Function] },
     _eventsCount: 1,
     _maxListeners: undefined,
     members: [] },
  _events: { event: [Function] },
  _eventsCount: 1,
  _maxListeners: undefined }
> myEmitter.on('event', ()=>{});
(node) warning: possible EventEmitter memory leak detected. 2 event listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at MyEmitter.addListener (events.js:239:17)
    at repl:1:11
    at REPLServer.defaultEval (repl.js:248:27)
    at bound (domain.js:287:14)
    at REPLServer.runBound [as eval] (domain.js:300:12)
    at REPLServer.<anonymous> (repl.js:412:12)
    at emitOne (events.js:82:20)
    at REPLServer.emit (events.js:169:7)
    at REPLServer.Interface._onLine (readline.js:210:10)
    at REPLServer.Interface._line (readline.js:549:8)
MyEmitter {
  domain: 
   Domain {
     domain: null,
     _events: { error: [Function] },
     _eventsCount: 1,
     _maxListeners: undefined,
     members: [] },
  _events: { event: [ [Function], [Function], warned: true ] },
  _eventsCount: 1,
  _maxListeners: undefined }
> 
Member

jasnell commented Dec 29, 2015

Same error does not occur using the repl:

bash-3.2$ node
> const EventEmitter = require('events');
undefined
> EventEmitter.defaultMaxListeners = 1
1
> class MyEmitter extends EventEmitter {}
[Function: MyEmitter]
> const myEmitter = new MyEmitter();
undefined
> myEmitter.on('event', ()=>{});
MyEmitter {
  domain: 
   Domain {
     domain: null,
     _events: { error: [Function] },
     _eventsCount: 1,
     _maxListeners: undefined,
     members: [] },
  _events: { event: [Function] },
  _eventsCount: 1,
  _maxListeners: undefined }
> myEmitter.on('event', ()=>{});
(node) warning: possible EventEmitter memory leak detected. 2 event listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at MyEmitter.addListener (events.js:239:17)
    at repl:1:11
    at REPLServer.defaultEval (repl.js:248:27)
    at bound (domain.js:287:14)
    at REPLServer.runBound [as eval] (domain.js:300:12)
    at REPLServer.<anonymous> (repl.js:412:12)
    at emitOne (events.js:82:20)
    at REPLServer.emit (events.js:169:7)
    at REPLServer.Interface._onLine (readline.js:210:10)
    at REPLServer.Interface._line (readline.js:549:8)
MyEmitter {
  domain: 
   Domain {
     domain: null,
     _events: { error: [Function] },
     _eventsCount: 1,
     _maxListeners: undefined,
     members: [] },
  _events: { event: [ [Function], [Function], warned: true ] },
  _eventsCount: 1,
  _maxListeners: undefined }
> 
@vkurchatkin

This comment has been minimized.

Show comment
Hide comment
@vkurchatkin

vkurchatkin Dec 29, 2015

Member

Reproduces with:

const EventEmitter = require('events');
EventEmitter.defaultMaxListeners = 1;
console.error('');

console.error triggers some events. Documentation states:

This class property lets you set it for all EventEmitter instances, current and future, effective immediately. Use with care.

So this seems kind of expected.

Member

vkurchatkin commented Dec 29, 2015

Reproduces with:

const EventEmitter = require('events');
EventEmitter.defaultMaxListeners = 1;
console.error('');

console.error triggers some events. Documentation states:

This class property lets you set it for all EventEmitter instances, current and future, effective immediately. Use with care.

So this seems kind of expected.

davidvgalbraith pushed a commit to davidvgalbraith/node that referenced this issue Dec 30, 2015

Dave
events: make sure console functions exist
if there's no global console cached, these calls will initialize it,
which in some cases can cause a circular dependency, making the console
received here an empty object. The program shouldn't crash in this case.

Fixes nodejs#4467
@davidvgalbraith

This comment has been minimized.

Show comment
Hide comment
@davidvgalbraith

davidvgalbraith Dec 30, 2015

So, node sets up the global objects e.g. console lazily to make startup really fast. The second call to myEmitter.on('event', ()=>{}); in your test.js wants to trigger console.error with that big message about max listeners detected. So it has to initialize the global console.
The way these initializations work is they store an empty object as the global object, then eval the relevant file which fills in the properties of this object when it modifies module.exports. This is to prevent an infinite loop if you have a circular dependency.
But console.js does module.exports = new Console(process.stdout, process.stderr);, which references process.stdout and process.stderr, causing a call to the stdout and stderr getters on lines 660 and 675 of src/node.js, which both call

      if ((stdout / stderr).isTTY) {
        process.on('SIGWINCH', function() {

. The second one of these wants to trigger another warning about max listeners detected, so it has to initialize the global console. But there's already such an initialization in progress, which has cached an empty object in the global console, so this empty object is returned as the global console. An empty object doesn't have an error method, so you get this stack trace.

There's no problem in the REPL because stdout and stderr aren't TTY, so these SIGWINCH events aren't added to process.

I propose a fix at #4479.

So, node sets up the global objects e.g. console lazily to make startup really fast. The second call to myEmitter.on('event', ()=>{}); in your test.js wants to trigger console.error with that big message about max listeners detected. So it has to initialize the global console.
The way these initializations work is they store an empty object as the global object, then eval the relevant file which fills in the properties of this object when it modifies module.exports. This is to prevent an infinite loop if you have a circular dependency.
But console.js does module.exports = new Console(process.stdout, process.stderr);, which references process.stdout and process.stderr, causing a call to the stdout and stderr getters on lines 660 and 675 of src/node.js, which both call

      if ((stdout / stderr).isTTY) {
        process.on('SIGWINCH', function() {

. The second one of these wants to trigger another warning about max listeners detected, so it has to initialize the global console. But there's already such an initialization in progress, which has cached an empty object in the global console, so this empty object is returned as the global console. An empty object doesn't have an error method, so you get this stack trace.

There's no problem in the REPL because stdout and stderr aren't TTY, so these SIGWINCH events aren't added to process.

I propose a fix at #4479.

@jasnell jasnell closed this in f9a59c1 Jan 15, 2016

evanlucas added a commit that referenced this issue Jan 18, 2016

events: make sure console functions exist
If there's no global console cached, initialize it.

Fixes: #4467
PR-URL: #4479
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>

scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016

Dave Michael Scovetta
events: make sure console functions exist
If there's no global console cached, initialize it.

Fixes: nodejs#4467
PR-URL: nodejs#4479
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment