Page MenuHomePhabricator

News story links are empty in featured feed
Closed, ResolvedPublic

Description

Happened yesterday (2/1) and again today (2/2).

Today's output:

"news": [
{
"links": [],
"story": "<!-- Jan 31 --> More than 49,000 men in <a rel=\"mw:WikiLink\" href=\"./England_and_Wales\" title=\"England and Wales\" id=\"mwBA\">England and Wales</a> are retroactively pardoned under the <b id=\"mwBQ\"><a rel=\"mw:WikiLink\" href=\"./Alan_Turing_law\" title=\"Alan Turing law\" id=\"mwBg\">Alan Turing law</a></b>, after being cautioned or convicted under <a rel=\"mw:WikiLink\" href=\"./Sodomy_law#United_Kingdom\" title=\"Sodomy law\" id=\"mwBw\">historical legislation that outlawed homosexual acts</a>."
},
{
"links": [],
"story": "<!-- Jan 31 --> <a rel=\"mw:WikiLink\" href=\"./Morocco\" title=\"Morocco\" id=\"mwCQ\">Morocco</a> is <a rel=\"mw:WikiLink\" href=\"./Member_states_of_the_African_Union\" title=\"Member states of the African Union\" id=\"mwCg\">readmitted</a> to the <b id=\"mwCw\"><a rel=\"mw:WikiLink\" href=\"./African_Union\" title=\"African Union\" id=\"mwDA\">African Union</a></b> despite an <a rel=\"mw:WikiLink\" href=\"./Political_status_of_Western_Sahara\" title=\"Political status of Western Sahara\" id=\"mwDQ\">ongoing dispute</a> over <a rel=\"mw:WikiLink\" href=\"./Western_Sahara\" title=\"Western Sahara\" id=\"mwDg\">Western Sahara</a>."
},
{
"links": [],
"story": "<!-- Jan 29 --> The <b id=\"mwEA\"><a rel=\"mw:WikiLink\" href=\"./Winter_X_Games_XXI\" title=\"Winter X Games XXI\" id=\"mwEQ\">Winter X Games</a></b> conclude with the United States winning seven gold medals <i id=\"mwEg\">(snowboarder <a rel=\"mw:WikiLink\" href=\"./Elena_Hight\" title=\"Elena Hight\" id=\"mwEw\">Elena Hight</a> pictured)</i>."
},
{
"links": [],
"story": "<!-- Jan 29 --> A gunman kills six people and injures eight others in <b id=\"mwFQ\"><a rel=\"mw:WikiLink\" href=\"./Quebec_City_mosque_shooting\" title=\"Quebec City mosque shooting\" id=\"mwFg\">a mass shooting</a></b> at a mosque in <a rel=\"mw:WikiLink\" href=\"./Quebec_City\" title=\"Quebec City\" id=\"mwFw\">Quebec City</a>, Canada."
},
{
"links": [],
"story": "<!-- Jan 26 --> <b id=\"mwGQ\"><a rel=\"mw:WikiLink\" href=\"./2017_North_India_cold_wave\" title=\"2017 North India cold wave\" id=\"mwGg\">A cold wave</a></b> affects <a rel=\"mw:WikiLink\" href=\"./North_India\" title=\"North India\" id=\"mwGw\">North India</a>."
}
]

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Mholloway renamed this task from News story links are empty in today's featured feed to News story links are empty in featured feed.Feb 2 2017, 3:01 PM
Mholloway updated the task description. (Show Details)
Mholloway removed a project: iOS-app-Bugs.

The MCS news endpoint is emitting merge links correctly. Might be a change in RESTBase.

https://appservice.wmflabs.org/en.wikipedia.org/v1/page/news

News link content is populating as expected in my local RESTBase. :|

http://localhost:7231/en.wikipedia.org/v1/feed/featured/2017/02/02:

...
"news": [
{
"links": [
{
"title": "Alan_Turing_law",
"extract": "The \"Alan Turing law\" is an informal term for the law in the United Kingdom, contained in the Policing and Crime Act 2017, which serves as an amnesty law to retroactively pardon men who were cautioned or convicted under historical legislation that outlawed homosexual acts. The provision is named after Alan Turing, the World War II codebreaker and computing pioneer, who was convicted for gross indecency in 1952. The law applies in England and Wales.\nSeveral proposals had been put forward for an Alan Turing law, and introducing such a law has been government policy since 2015. To implement the pardon, the British Government announced on 20 October 2016 that it would support an amendment to the Policing and Crime Bill that would provide a posthumous pardon for the dead, and also provide an automatic formal pardon for living people who had had such offences removed from their record.",
"thumbnail": {
"source": "https://upload.wikimedia.org/wikipedia/commons/0/07/Alan_Turing_portr%C3%A9.jpg",
"width": 280,
"height": 309,
"original": "http://upload.wikimedia.org/wikipedia/commons/0/07/Alan_Turing_portr%C3%A9.jpg"
},
"originalimage": {
"source": "https://upload.wikimedia.org/wikipedia/commons/0/07/Alan_Turing_portr%C3%A9.jpg",
"width": 280,
"height": 309
},
"lang": "en",
"dir": "ltr",
"timestamp": "2017-02-02T14:32:19Z",
"normalizedtitle": "Alan Turing law"
},
{
"title": "England_and_Wales",
"extract": "England and Wales (Welsh: Cymru a Lloegr) is a legal jurisdiction covering England and Wales, two of the four countries of the United Kingdom, which form the constitutional successor to the former Kingdom of England and follow a single legal system, known as English law.\nThe devolved National Assembly for Wales (Welsh: Cynulliad Cenedlaethol Cymru) was created in 1999 by the Parliament of the United Kingdom under the Government of Wales Act 1998 and provides a degree of self-government in Wales. The powers of the Assembly were expanded by the Government of Wales Act 2006, which allows it to pass its own laws, and the Act also formally separated the Welsh Government from the Assembly. There is no equivalent body for England, which is directly governed by the Parliament and the government of the United Kingdom.",
"thumbnail": {
"source": "https://upload.wikimedia.org/wikipedia/commons/thumb/b/b6/England_and_Wales_within_the_UK_and_Europe.svg/320px-England_and_Wales_within_the_UK_and_Europe.svg.png",
"width": 320,
"height": 245,
"original": "http://upload.wikimedia.org/wikipedia/commons/b/b6/England_and_Wales_within_the_UK_and_Europe.svg"
},
"originalimage": {
"source": "https://upload.wikimedia.org/wikipedia/commons/b/b6/England_and_Wales_within_the_UK_and_Europe.svg",
"width": 680,
"height": 520
},
"lang": "en",
"dir": "ltr",
"timestamp": "2017-01-18T17:48:02Z",
"description": "administrative jurisdiction within the United Kingdom",
"normalizedtitle": "England and Wales"
},
{
"title": "Sodomy_law",
"extract": "A sodomy law is a law that defines certain sexual acts as crimes. The precise sexual acts meant by the term sodomy are rarely spelled out in the law, but are typically understood by courts to include any sexual act deemed to be \"unnatural\" or immoral. Sodomy typically includes anal sex, oral sex and bestiality. In practice, sodomy laws have rarely been enforced against heterosexual couples.\nAs of August 2016, 72 countries as well as five sub-national jurisdictions have laws criminalizing homosexuality, with most of them located in Asia and Africa.",
"lang": "en",
"dir": "ltr",
"timestamp": "2017-01-28T05:25:58Z",
"description": "laws criminalising certain sexual acts",
"normalizedtitle": "Sodomy law"
}
],
"story": "<!-- Jan 31 --> More than 49,000 men in <a rel=\"mw:WikiLink\" href=\"./England_and_Wales\" title=\"England and Wales\" id=\"mwBA\">England and Wales</a> are retroactively pardoned under the <b id=\"mwBQ\"><a rel=\"mw:WikiLink\" href=\"./Alan_Turing_law\" title=\"Alan Turing law\" id=\"mwBg\">Alan Turing law</a></b>, after being cautioned or convicted under <a rel=\"mw:WikiLink\" href=\"./Sodomy_law#United_Kingdom\" title=\"Sodomy law\" id=\"mwBw\">historical legislation that outlawed homosexual acts</a>."
},
...
]

...and $merge links are emitted as expected on scb:

mholloway-shell@scb1001:~$ curl http://localhost:8888/en.wikipedia.org/v1/page/news

[  
   {  
      "links":[  
         {  
            "$merge":[  
               "http://restbase.svc.eqiad.wmnet:7231/en.wikipedia.org/v1/page/summary/wiki%2FAlan_Turing_law"
            ]
         },
         {  
            "$merge":[  
               "http://restbase.svc.eqiad.wmnet:7231/en.wikipedia.org/v1/page/summary/wiki%2FEngland_and_Wales"
            ]
         },
         {  
            "$merge":[  
               "http://restbase.svc.eqiad.wmnet:7231/en.wikipedia.org/v1/page/summary/wiki%2FSodomy_law"
            ]
         }
      ],
      "story":"<!-- Jan 31 --> More than 49,000 men in <a rel=\"mw:WikiLink\" href=\"./England_and_Wales\" title=\"England and Wales\" id=\"mwBA\">England and Wales</a> are retroactively pardoned under the <b id=\"mwBQ\"><a rel=\"mw:WikiLink\" href=\"./Alan_Turing_law\" title=\"Alan Turing law\" id=\"mwBg\">Alan Turing law</a></b>, after being cautioned or convicted under <a rel=\"mw:WikiLink\" href=\"./Sodomy_law#United_Kingdom\" title=\"Sodomy law\" id=\"mwBw\">historical legislation that outlawed homosexual acts</a>."
   }
}
...
mobrovac triaged this task as High priority.
mobrovac subscribed.

We deployed a small change to the feeds yesterday in order to accommodate the future announcements feed endpoint. I suspect this is the culprit. In order to minimise the impact, I will revert it now and then investigate later what went wrong.

Update for posterity:

...and $merge links are emitted as expected on scb:

... is incorrect. See the inappropriate "wiki%2F" prefixing the title in the $merge link in production:

"links":[  
   {  
      "$merge":[  
         "http://restbase.svc.eqiad.wmnet:7231/en.wikipedia.org/v1/page/summary/wiki%2FAlan_Turing_law"
      ]
   },

Oddly, this doesn't happen locally or on https://appservice.wmflabs.org/en.wikipedia.org/v1/page/news.

It seems like this change caused news article link hrefs to be prefixed when they weren't before, causing problems here when they were used to construct a $merge link.

Still not sure why we were only seeing this in production.

Note: the prefixes were also getting rewritten by _rewriteUrlAttribute but the result would have been broken either way.

It's not occurring right now in prod since prod has been reverted to an earlier deploy commit after we noticed this issue.

Looks like this issue was due to, what I believe to be, an incorrect usage of domino in news.js line 25.

const href = anchor.href;

Changing this to anchor.getAttribute('href') makes the issue go away. Will look in the code base to see if I can find more of these occurrences.

I suspect that this is due to the upgrade of domino from 1.0.27 to 1.0.28. There have been plenty of changes there. It would explain why I was able to repro the issue in some git clones of MCS, even with much older revisions, but not on others.

mobrovac lowered the priority of this task from High to Medium.

Thank you @bearND for the investigation! Lowering the priority now that it is not affecting production.

Ok, I think the puzzle is solved. I wrote a short script

const domino = require('domino');
const linkHtml = `<html><head><base href="//en.wikipedia.org/wiki/"/>
</head></html><body>
<a rel="mw:WikiLink" href="./my_href" title="my title" id="mwEg">foo bar</a>
</body></html>`;
const document = domino.createDocument(linkHtml);
const link = document.querySelector('a');
console.log(link.href);

ran it against npm install domino@1.0.27 --save and npm install domino@1.0.28 --save.

1.0.27: /my_href
1.0.28: /wiki/my_href

As @Arlolra explained in the #mediawiki-parsoid channel today the href property resolves the URI. I guess 1.0.28 fixed it to take the base URI into consideration.
In our code we need to search for occurrences of this syntax to make sure it does what we expect it to do.

Yes, the change in lib/Document.js looks like it. I think domino is correct. We just need to adjust our usage of it. Maybe other users of domino are affected, too?

I did a cursory audit of Parsoid's uses w/ git grep "\.href", just in case.

Change 335745 had a related patch set uploaded (by BearND):
Fix news links

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

Change 337515 had a related patch set uploaded (by BearND):
On this day: fix summary links

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

Change 337515 merged by jenkins-bot:
On this day: fix summary links

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