Page MenuHomePhabricator

mw.trackSubscribe should prevent infinite recursion
Closed, DeclinedPublic

Description

T69420 was apparently caused by a deprecation warning from inside the handler that attempts to log deprecation warnings. While 1e9adcfc510234a0c should fix the immediate problem in that bug, it would probably be a good idea for ext.wikimediaEvents.deprecate.js to bail out when called recursively to avoid a repeat next time.

Details

Reference
bz67481

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:31 AM
bzimport set Reference to bz67481.
bzimport added a subscriber: Unknown Object (MLST).

Detecting recursive calls might be difficult, but it can just suppress all warnings. From /tests/qunit/data/testrunner.js in core, these functions are used several times in tests:

var warn;

function suppressWarnings() {

		warn = mw.log.warn;
		mw.log.warn = $.noop;

}

function restoreWarnings() {

		if ( warn !== undefined ) {
			mw.log.warn = warn;
			warn = undefined;
		}

}

The high priority bug was T69420 which has been fixed.

Actually, I don't see any way to implement this, given that everything can be asynchronous. I might be missing something, but this seems impossible.

We can implement some half-solutions (only guarantee detection of this scenario when both the track handler and everything it does is synchronous; stop sending further tracks after some limit is reached;), but they would be prone to failure and could instill a false sense of confidence.

There is a (conceptually) very simple way to avoid this: don't use deprecated APIs in your mw.track handler.

matmarex claimed this task.

Since nobody challenged my opinion above, I take it that you all agree with me.

Krinkle set Security to None.
Krinkle updated the task description. (Show Details)
Krinkle removed a subscriber: Unknown Object (MLST).