🚨 Error: Cannot read property 'length' of null

[ampprojectbot]

Details

Error report: link
First seen: Apr 9, 2020
Frequency: ~ 455,777/day

Stacktrace

Error: Cannot read property 'length' of null
    at url (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/service/url-expander/expander.js:75:9)
    at expand (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/service/url-replacements-impl.js:889:44)
    at urlReplacements (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/extensions/amp-social-share/0.1/amp-social-share.js:141:11)
    at layoutCallback (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/custom-element.js:1153:39)
    at fn (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/utils/promise.js:75:12)
    at https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/utils/promise.js:74:9
    at tryResolve (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/custom-element.js:1153:22)
    at layoutCallback (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/service/resource.js:908:18)
    at callback (https://raw.githubusercontent.com/ampproject/amphtml/2005050322000/src/service/vsync-impl.js:470:16)

Notes

@calebcordry modified src/service/url-expander/expander.js:75-75 in #12682 (Jan 20, 2018)
@rsimha modified src/service/url-replacements-impl.js:889-893 in #21212 (May 16, 2019)
@caroqliu modified extensions/amp-social-share/0.1/amp-social-share.js:141-160 in #28100 (Apr 30, 2020)
@zhouyx modified src/custom-element.js:1152-1153 in #15410 (May 18, 2018)
@jridgewell modified src/utils/promise.js:75-78 in #15143 (May 8, 2018)
@zhouyx modified src/custom-element.js:1152-1153 in #15410 (May 18, 2018)
@jridgewell modified src/service/resource.js:905-913 in #20814 (Feb 14, 2019)
@jridgewell modified src/service/vsync-impl.js:470-471 in #20836 (Feb 14, 2019)

Seen in:

• 05-05 Experimental (0322)
• 04-25 Stable (2135)

Possible assignees: @jridgewell, @zhouyx

/cc @ampproject/release-on-duty

[rcebulko]

/cc @caroqliu This seems related to #28100, think you could take a peek?

[rcebulko]

Note: This has existed in other forms for a while, but it spiked a bunch when the latest Experimental was promoted on 05-06 this week. Since it's mostly appearing in Experimental, this is probably related to some experiment being run

[rcebulko]

[jridgewell]

It's because social-share does an early return in buildCallback, and does toggle(false) in those cases to make it display: none. But, making an element display: none doesn't always prevent the element from receiving layoutCallback. (I'm looking into why this is.)

When this early return happens, there's no shareEndpoint_ value set. And we then call expandUrlAsync with a null value.

Instead, we should throw an error in buildCallback. That should prevent layoutCallback from firing in all cases.

/to @caroqliu

[caroqliu]

Until the layoutCallback bug is resolved -- I'd prefer to directly check shareEndpoint_ or element visibility in the layoutCallback to early return instead of calling expandUrlAsync. I'm not sure I agree with throwing a different error earlier to avoid this error later.

[rcebulko]

^^^ +1

[rcebulko]

Great work taking care of this so quickly!