Amphtml: timeoutMillis RTC config is ignored when using single pass compilation

[philipwatson]

timeoutMillis RTC config is ignored

Using a dist build and custom url callout. Latest master (6a8eb2c).

I noticed the ad network is called immediately around the same time the callout is made.

The default timeout is 1 second. AMP doesn't wait for the callout response or this timeout.

Problem still exists if I set the timeoutMillis on the ad's rtc-config attribute.

I think I may of found the problem:

In real-time-config-manager.js.

inflateAndSendRtc_(url, macros, errorReportingUrl, opt_vendor) {
    let {timeoutMillis} = this.rtcConfig_;
    //...

timeoutMillis will be undefined here (when debugging).

Possible fix:

inflateAndSendRtc_(url, macros, errorReportingUrl, opt_vendor) {
    let timeoutMillis = this.rtcConfig_['timeoutMillis'];
    //...

This fixes the problem. Though you guys may have a better solution.

Looks like the build is minifying the timeoutMillis variable.. thus the destructuring will not work.

Somehow even without the destructuring it will not work:

inflateAndSendRtc_(url, macros, errorReportingUrl, opt_vendor) {
    let timeoutMillis = this.rtcConfig_.timeoutMillis;
    //...

[choumx]

/cc @ampproject/wg-ads

[philipwatson]

It is only an issue if using the --single_pass flag when building the dist.

It also minifies the Doubleclick's fields on their custom RTC config macro object. So none of their macros, such as CANONICAL_URL, is substituted on the callout URL.

Only the macros that are common across all ad-networks are substituted (TIMEOUT, CONSENT_STATE and CONSENT_STRING) because of the way there are coded: macros['TIMEOUT'] = ... instead of macros.TIMEOUT = ....

There could be other undesired object key minification.

[zhouyx]

@philipwatson Thanks for reporting. Good thing is that we haven't shipped the single pass compilation yet.

I believe what you found is the exact cause for this issue, and your suggestion to fix it sounds right. It'd be great if you can contribute and submit a fix to the issue.

cc @erwinmombay @lannka

[erwinmombay]

i believe i fixed this here https://github.com/ampproject/amphtml/pull/24453/files#diff-fe38850932308f7278208f632fed8d98R607 (not yet merged, will try to finish it today)

[erwinmombay]

single pass project is obsolete closing.