Page MenuHomePhabricator
Create Task
Maniphest T217093

Cannot read property 'docId' of undefined
Closed, ResolvedPublic
Actions

Description

Deployed was reverted due to these beauties.

https://logstash.wikimedia.org/app/kibana#/doc/logstash-*/logstash-2019.02.25/parsoid?id=AWkmgNN_1DNFq-AXi5N7&_g=h@44136fa

TypeError: Cannot read property 'docId' of undefined
    at Function.setNodeData (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/utils/DOMDataUtils.js:48:27)
    at Function.getNodeData (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/utils/DOMDataUtils.js:37:9)
    at Function.getDataParsoid (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/utils/DOMDataUtils.js:55:19)
    at ConversionTraverser.DOMTraverser.traverse (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/utils/DOMTraverser.js:122:24)
    at ConversionTraverser.DOMTraverser.traverse (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/utils/DOMTraverser.js:140:10)
    at ConversionTraverser.DOMTraverser.traverse (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/utils/DOMTraverser.js:140:10)
    at ConversionTraverser.DOMTraverser.traverse (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/utils/DOMTraverser.js:140:10)
    at Function.baseToVariant (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/language/LanguageConverter.js:457:5)
    at Function.maybeConvert (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/language/LanguageConverter.js:405:8)
    at Object.apiUtils.languageConversion (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/api/apiUtils.js:601:21)
    at /srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/src/lib/api/routes.js:636:21
    at next (native)
    at tryCatchNext (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/node_modules/prfun/lib/index.js:783:28)
    at continuer (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/node_modules/prfun/lib/index.js:800:24)
    at callback (/srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/node_modules/prfun/lib/index.js:812:43)
    at /srv/deployment/parsoid/deploy-cache/revs/cb62482a8f25931b86757b3ff26f8e9565f4005d/node_modules/prfun/lib/index.js:814:9

Details

SubjectRepoBranchLines +/-
Use env.createDocument in lib/api/apiUtils.jsmediawiki/services/parsoidmaster+7 -6
Customize query in gerrit

Related Objects

Event Timeline

Arlolra created this task.Feb 25 2019, 9:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 25 2019, 9:28 PM
Arlolra added a project: Parsoid.Feb 25 2019, 9:28 PM
Arlolra claimed this task.Feb 25 2019, 9:34 PM
Arlolra triaged this task as Medium priority.
Arlolra mentioned this in T217095: Aborted deploy does not revert config changes.
ssastry added subscribers: cscott, ssastry.Feb 25 2019, 9:49 PM

So, our pre-deploy testing has a hole for lang variants since rt testing doesn't cover this feature. Presumably, this would have been caught on beta cluster but that is currently readonly. In any case, @cscott, this is a testing gap currently.

gerritbot subscribed.Feb 25 2019, 9:54 PM

Change 492776 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Use env.createDocument in lib/api/apiUtils.js

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

gerritbot added a project: Patch-For-Review.Feb 25 2019, 9:54 PM
Arlolra added a comment.Feb 25 2019, 9:59 PM

In any case, @cscott, this is a testing gap currently.

Lots of tests mocha were introduced in,
https://github.com/wikimedia/parsoid/commit/4f298233cc95b823c34d605a08f109cf5b1a4157

but it looks like this only would have been caught if the DOMTraverser was given something with encapsulation,
https://github.com/wikimedia/parsoid/blob/master/lib/utils/DOMTraverser.js#L122

gerritbot added a comment.Feb 25 2019, 10:07 PM

Change 492776 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Use env.createDocument in lib/api/apiUtils.js

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

ssastry added a comment.Feb 25 2019, 10:07 PM
In T217093#4982822, @Arlolra wrote:

In any case, @cscott, this is a testing gap currently.

Lots of tests mocha were introduced in,
https://github.com/wikimedia/parsoid/commit/4f298233cc95b823c34d605a08f109cf5b1a4157

but it looks like this only would have been caught if the DOMTraverser was given something with encapsulation,
https://github.com/wikimedia/parsoid/blob/master/lib/utils/DOMTraverser.js#L122

That is fixable, but I also meant more generally. RT testing exercises our code more thoroughly .. so, we need an equivalent for lang variants that pushes real pages through the endpoints. I think scott did have a plan for that, but postponed for when we consider the lang variant endpoints non-alpha.

Arlolra closed this task as Resolved.Feb 25 2019, 10:18 PM
Arlolra mentioned this in T217098: Testing infra that pushes real pages through the language converter endpoint.
Stashbot mentioned this in T204608: Use a bag-on-the-side implementation, rather than an internal .dataobject for node data.Feb 26 2019, 6:24 PM
Stashbot mentioned this in T214099: Stress test Parsoid's HTTP API.
Stashbot subscribed.

Mentioned in SAL (#wikimedia-operations) [2019-02-26T18:24:33Z] <arlolra> Updated Parsoid to e82347d (T204608, T214099, T217093)

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL