Page MenuHomePhabricator

ext.wikimediaEvents.searchSuggest missing dependency (TypeError: mw.searchSuggest is undefined)
Closed, DeclinedPublic

Description

A user reported the following JS error:

Exception in module-execute in module ext.wikimediaEvents: load.php:177:688
TypeError: mw.searchSuggest is undefined TypeError: mw.searchSuggest is undefined

And ext.wikimediaEvents.searchSuggest.js really uses mw.searchSuggest without declaring a dependency on mediawiki.searchSuggest.

Event Timeline

Schnark created this task.Sep 29 2015, 7:30 AM
Schnark raised the priority of this task from to Needs Triage.
Schnark updated the task description. (Show Details)
Schnark added a subscriber: Schnark.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 29 2015, 7:30 AM
  1. mw.user.generateRandomSessionId() is referenced in line 11.
    1. This might be always the case at this point of time.
    2. As the error message above shows, one cannot rely on such assumptions.
    3. It would be more clean to declare all dependencies with .using() first.
  2. In line 40 mw.searchSuggest.request is used.
    1. If this is expected to have been set before in the outer world, that should be made explicit here.
    2. In line 58 this component is defined.
    3. If that is supposed to overwrite a previous assignment that should be stated now.
    4. If not, line 40 is assigning an undefined as callback.
  1. mw.user.generateRandomSessionId() is referenced in line 11.
    1. This might be always the case at this point of time.
    2. As the error message above shows, one cannot rely on such assumptions.
    3. It would be more clean to declare all dependencies with .using() first.

The dependency on mediawiki.user is declared in WikimediaEvents.php, no need to use .using().

The dependency on mediawiki.user is declared in WikimediaEvents.php, no need to use .using().

Okay; a small comment on that assumption would be helpful anyway. The context when and where this snippet is running is not documented.

EBernhardson added a subscriber: EBernhardson.EditedOct 17 2015, 6:14 PM

The context this is run in is also documented in WikimediaEvents.php. This code is loaded globally for all users (including logged-out). I'm not sure what the best way is to force this code to depend on mediawiki.searchSuggest being loaded, the dependencies go in the other way (we would have to force loading on pages where it wasn't needed). On the other hand though, the only time mediawiki.searchSuggest is not loaded is when $wgUseAjax or $wgEnableApi are disabled, and it should be safe to assume these will always be enabled on WMF production sites.

'ext.wikimediaEvents' => array(
    // Loaded globally for all users (including logged-out)
    // Don't remove if empty!
    'scripts'       => array(
        'ext.wikimediaEvents.resourceloader.js',
        'ext.wikimediaEvents.searchSuggest.js',
        'ext.wikimediaEvents.statsd.js',
        'ext.wikimediaEvents.search.js',
    ),
    'dependencies' => array(
        'mediawiki.user', // needed by searchSuggest.js
        'mediawiki.Uri', // needed by search.js
    ),
    'localBasePath' => __DIR__ . '/modules',
    'remoteExtPath' => 'WikimediaEvents/modules',
    'targets' => array( 'desktop', 'mobile' ),
),

You can assume that mediawiki.searchSuggest will be loaded on any page, but you cannot assume, that it is already loaded when ext.wikimediaEvents is running. Therefor you need a dependency in your Resource.php definition or a mediawiki.loader.using in the module to ensure that the needed module is loaded *and* ready to run.

Just assuming a module is loaded on a page is nice, but in a async load order this is not the best practise.

Umherirrender set Security to None.
dg711 added a subscriber: dg711.Jan 11 2016, 7:47 PM
Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptJan 11 2016, 7:47 PM
IoannisKydonis added a subscriber: IoannisKydonis.

I will do it.

Change 266019 had a related patch set uploaded (by IoannisKydonis):
Add dependency for ext.wikimediaEvents.search.js

https://gerrit.wikimedia.org/r/266019

Krinkle renamed this task from Missing dependency for ext.wikimediaEvents.searchSuggest.js to ext.wikimediaEvents.searchSuggest missing dependency (TypeError: mw.searchSuggest is undefined).Jan 23 2016, 11:13 PM
Krinkle triaged this task as Unbreak Now! priority.
Krinkle added a project: Regression.
Krinkle edited projects, added Wikimedia-production-error; removed Regression.

This task has been "Unbreak now" priority for nearly two weeks...

The patch in Gerrit needs rework. Is anybody willing to work on it?

This task has been "Unbreak now" priority for five weeks.

The patch in Gerrit needs rework. Is anybody willing to work on it?

The extension WikimediaEvents doesn't have ext.wikimediaEvents.searchSuggest.js any more.
Also ext.wikimediaEvents.search.js and ext.wikimediaEvents.searchSatisfaction.js only adds a subscriber to 'mediawiki.searchSuggest' event.
We need to check if the issue still exists.

Krinkle closed this task as Declined.Mar 16 2016, 8:43 PM
Krinkle added a subscriber: Krinkle.

Doesn't seem to be an issue any more. Adding the dependency is unlikely to solve anything as the module doesn't use searchSuggest. Hook subscription is static and doesn't require the emitting module to be loaded first.

Change 266019 abandoned by Krinkle:
Add dependency for ext.wikimediaEvents.search.js

https://gerrit.wikimedia.org/r/266019

Restricted Application added subscribers: Luke081515, TerraCodes, Urbanecm. · View Herald TranscriptMay 7 2016, 6:31 PM