Don't swallow errors in URL replacements promises.

And unifies the behavior across sync and non-sync replacements.
Both now replace with the empty string and report the error to the console and the server where applicable. It would have been an option to fail the URL generation instead (previous that was the behavior for sync errors, but I believe that erring on the side of producing a URL is the right thing to do.

Fixes #2418

src/url-replacements.js

@@ -21,16 +21,13 @@ import {documentInfoFor} from './document-info';
 import {getMode} from './mode';
 import {getService} from './service';
 import {loadPromise} from './event-helper';
-import {log} from './log';
 import {getSourceUrl, parseUrl, removeFragment, parseQueryString} from './url';
 import {viewerFor} from './viewer';
 import {viewportFor} from './viewport';
 import {vsyncFor} from './vsync';
 import {userNotificationManagerFor} from './user-notification';
 import {activityFor} from './activity';
 
-/** @private {string} */
-const TAG_ = 'UrlReplacements';
 
 /**
  * This class replaces substitution variables with their values.
@@ -388,15 +385,28 @@ class UrlReplacements {
       }
       const binding = (opt_bindings && (name in opt_bindings)) ?
           opt_bindings[name] : this.getReplacement_(name);
-      const val = (typeof binding == 'function') ?
-          binding.apply(null, args) : binding;
+      let val;
+      try {
+        val = (typeof binding == 'function') ?
+            binding.apply(null, args) : binding;
+      } catch (e) {
+        // Report error, but do not disrupt URL replacement. This will
+        // interpolate as the empty string.
+        setTimeout(() => {
+          throw e;
+        });
+      }
       // In case the produced value is a promise, we don't actually
       // replace anything here, but do it again when the promise resolves.
       if (val && val.then) {
-        const p = val.then(v => {
+        const p = val.catch(err => {
+          // Report error, but do not disrupt URL replacement. This will
+          // interpolate as the empty string.
+          setTimeout(() => {
+            throw err;
+          });
+        }).then(v => {
           url = url.replace(match, encodeValue(v));
-        }, err => {
-          log.error(TAG_, 'Failed to expand: ' + name, err);
         });
         if (replacementPromise) {
           replacementPromise = replacementPromise.then(() => p);

test/functional/test-url-replacements.js

@@ -366,6 +366,33 @@ describe('UrlReplacements', () => {
         .to.eventually.equal('?a=b&b=b');
   });
 
+  it('should report errors & replace them with empty string (sync)', () => {
+    const clock = sandbox.useFakeTimers();
+    const replacements = urlReplacementsFor(window);
+    replacements.set_('ONE', () => {
+      throw new Error('boom');
+    });
+    const p = expect(replacements.expand('?a=ONE')).to.eventually.equal('?a=');
+    expect(() => {
+      clock.tick(1);
+    }).to.throw(/boom/);
+    return p;
+  });
+
+  it('should report errors & replace them with empty string (promise)', () => {
+    const clock = sandbox.useFakeTimers();
+    const replacements = urlReplacementsFor(window);
+    replacements.set_('ONE', () => {
+      return Promise.reject(new Error('boom'));
+    });
+    return expect(replacements.expand('?a=ONE')).to.eventually.equal('?a=')
+        .then(() => {
+          expect(() => {
+            clock.tick(1);
+          }).to.throw(/boom/);
+        });
+  });
+
   it('should support positional arguments', () => {
     const replacements = urlReplacementsFor(window);
     replacements.set_('FN', one => one);