ampproject · jridgewell · Jun 6, 2017 · May 25, 2017 · May 25, 2017 · May 26, 2017
diff --git a/build-system/amp4test.js b/build-system/amp4test.js
@@ -31,7 +31,7 @@ app.get('/compose-doc', function(req, res) {
 <body>
 ${req.query.body}
 </body>
-</html>  
+</html>
 `);
 });
 

diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js
@@ -1215,7 +1215,7 @@ export class AmpA4A extends AMP.BaseElement {
           // across pages.
           ampCors: false,
           credentials: 'omit',
-        }).then(jwkSetObj => {
+        }).then(res => res.json()).then(jwkSetObj => {
           const result = {serviceName: currServiceName};
           if (isObject(jwkSetObj) && Array.isArray(jwkSetObj.keys) &&
               jwkSetObj.keys.every(isObject)) {

diff --git a/extensions/amp-a4a/0.1/test/test-a4a-integration.js b/extensions/amp-a4a/0.1/test/test-a4a-integration.js
@@ -101,8 +101,11 @@ describe('integration test: a4a', () => {
           method: 'GET',
           ampCors: false,
           credentials: 'omit',
-        }).returns(
-          Promise.resolve({keys: [JSON.parse(validCSSAmp.publicKey)]}));
+        }).returns(Promise.resolve({
+          json() {
+            return Promise.resolve({keys: [JSON.parse(validCSSAmp.publicKey)]});
+          },
+        }));
     }
     // Expect ad request.
     headers = {};

diff --git a/extensions/amp-a4a/0.1/test/test-amp-a4a.js b/extensions/amp-a4a/0.1/test/test-amp-a4a.js
@@ -92,8 +92,11 @@ describe('amp-a4a', () => {
         method: 'GET',
         ampCors: false,
         credentials: 'omit',
-      }).returns(
-        Promise.resolve({keys: [JSON.parse(validCSSAmp.publicKey)]}));
+      }).returns(Promise.resolve({
+        json() {
+          return Promise.resolve({keys: [JSON.parse(validCSSAmp.publicKey)]});
+        },
+      }));
     viewerWhenVisibleMock = sandbox.stub(Viewer.prototype, 'whenFirstVisible');
     viewerWhenVisibleMock.returns(Promise.resolve());
     mockResponse = {
@@ -2095,8 +2098,11 @@ describe('amp-a4a', () => {
             method: 'GET',
             ampCors: false,
             credentials: 'omit',
-          }).returns(
-          Promise.resolve({keys: [testKey, testKey, testKey]}));
+          }).returns(Promise.resolve({
+            json() {
+              return Promise.resolve({keys: [testKey, testKey, testKey]});
+            },
+          }));
       expect(win.ampA4aValidationKeys).not.to.exist;
       // Key fetch happens on A4A class construction.
       const a4a = new MockA4AImpl(a4aElement);  // eslint-disable-line no-unused-vars
@@ -2133,8 +2139,13 @@ describe('amp-a4a', () => {
             method: 'GET',
             ampCors: false,
             credentials: 'omit',
-          }).returns(
-              Promise.resolve({keys: [JSON.parse(validCSSAmp.publicKey)]}));
+          }).returns(Promise.resolve({
+            json() {
+              return Promise.resolve({keys: [
+                JSON.parse(validCSSAmp.publicKey),
+              ]});
+            },
+          }));
       expect(win.ampA4aValidationKeys).not.to.exist;
       // Key fetch happens on A4A class construction.
       const a4a = new MockA4AImpl(a4aElement);  // eslint-disable-line no-unused-vars
@@ -2165,7 +2176,11 @@ describe('amp-a4a', () => {
             method: 'GET',
             ampCors: false,
             credentials: 'omit',
-          }).returns(Promise.resolve({keys: ['invalid key data']}));
+          }).returns(Promise.resolve({
+            json() {
+              return Promise.resolve({keys: ['invalid key data']});
+            },
+          }));
       expect(win.ampA4aValidationKeys).not.to.exist;
       // Key fetch happens on A4A class construction.
       const a4a = new MockA4AImpl(a4aElement);  // eslint-disable-line no-unused-vars

diff --git a/extensions/amp-access-laterpay/0.1/laterpay-impl.js b/extensions/amp-access-laterpay/0.1/laterpay-impl.js
@@ -196,17 +196,21 @@ export class LaterpayVendor {
       this.emptyContainer_();
       return {access: response.access};
     }, err => {
-      const status = err && err.response && err.response.status;
-      if (status === 402) {
-        this.purchaseConfig_ = err.responseJson;
+      if (!err || !err.response) {
+        throw err;
+      }
+      const {response} = err;
+      if (response.status !== 402) {
+        throw err;
+      }
+      return response.json().catch(() => undefined).then(responseJson => {
+        this.purchaseConfig_ = responseJson;
         // empty before rendering, in case authorization is being called again
         // with the same state
         this.emptyContainer_()
           .then(this.renderPurchaseOverlay_.bind(this));
-      } else {
-        throw err;
-      }
-      return {access: false};
+        return {access: false};
+      });
     });
   }
 
@@ -227,7 +231,7 @@ export class LaterpayVendor {
           AUTHORIZATION_TIMEOUT,
           this.xhr_.fetchJson(url, {
             credentials: 'include',
-          }));
+          })).then(res => res.json());
     });
   }
 

diff --git a/extensions/amp-access-laterpay/0.1/test/test-amp-access-laterpay.js b/extensions/amp-access-laterpay/0.1/test/test-amp-access-laterpay.js
@@ -91,7 +91,11 @@ describes.fakeWin('LaterpayVendor', {
         .withExactArgs('https://builturl', {
           credentials: 'include',
         })
-        .returns(Promise.resolve({access: true}))
+        .returns(Promise.resolve({
+          json() {
+            return Promise.resolve({access: true});
+          },
+        }))
         .once();
       return vendor.authorize().then(resp => {
         expect(resp.access).to.be.true;
@@ -129,8 +133,12 @@ describes.fakeWin('LaterpayVendor', {
           credentials: 'include',
         })
         .returns(Promise.reject({
-          response: {status: 402},
-          responseJson: {access: false},
+          response: {
+            status: 402,
+            json() {
+              return Promise.resolve({access: false});
+            },
+          },
         }))
         .once();
       emptyContainerStub.returns(Promise.resolve());

diff --git a/extensions/amp-access/0.1/amp-access-client.js b/extensions/amp-access/0.1/amp-access-client.js
@@ -123,10 +123,10 @@ export class AccessClientAdapter {
     return urlPromise.then(url => {
       dev().fine(TAG, 'Authorization URL: ', url);
       return this.timer_.timeoutPromise(
-          this.authorizationTimeout_,
-          this.xhr_.fetchJson(url, {
-            credentials: 'include',
-          }));
+        this.authorizationTimeout_,
+        this.xhr_.fetchJson(url, {
+          credentials: 'include',
+        })).then(res => res.json());
     });
   }
 

diff --git a/extensions/amp-access/0.1/test/test-amp-access-client.js b/extensions/amp-access/0.1/test/test-amp-access-client.js
@@ -159,7 +159,11 @@ describes.realWin('AccessClientAdapter', {
             .withExactArgs('https://acme.com/a?rid=reader1', {
               credentials: 'include',
             })
-            .returns(Promise.resolve({access: 'A'}))
+            .returns(Promise.resolve({
+              json() {
+                return Promise.resolve({access: 'A'});
+              },
+            }))
             .once();
         return adapter.authorize().then(response => {
           expect(response).to.exist;

diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-doubleclick-integration.js b/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-doubleclick-integration.js
@@ -51,7 +51,11 @@ describes.sandboxed('A4A integration', {}, () => {
     it.skip('should send ping beacons for all lifecycle stages', () => {
       pingStub.returns(null);
       creativeXhrStub.returns(Promise.resolve({}));
-      keyXhrStub.returns(Promise.resolve({}));
+      keyXhrStub.returns(Promise.resolve({
+        json() {
+          return Promise.resolve({});
+        },
+      }));
       expect(0).to.equal(1);
     });
   });

diff --git a/extensions/amp-ad/0.1/amp-ad-custom.js b/extensions/amp-ad/0.1/amp-ad-custom.js
@@ -83,7 +83,8 @@ export class AmpAdCustom extends AMP.BaseElement {
     // If this promise has no URL yet, create one for it.
     if (!(fullUrl in ampCustomadXhrPromises)) {
       // Here is a promise that will return the data for this URL
-      ampCustomadXhrPromises[fullUrl] = xhrFor(this.win).fetchJson(fullUrl);
+      ampCustomadXhrPromises[fullUrl] = xhrFor(this.win).fetchJson(fullUrl)
+          .then(res => res.json());
     }
     return ampCustomadXhrPromises[fullUrl].then(data => {
       const element = this.element;

diff --git a/extensions/amp-analytics/0.1/amp-analytics.js b/extensions/amp-analytics/0.1/amp-analytics.js
@@ -357,6 +357,7 @@ export class AmpAnalytics extends AMP.BaseElement {
           remoteConfigUrl = expandedUrl;
           return xhrFor(ampdoc.win).fetchJson(remoteConfigUrl, fetchConfig);
         })
+        .then(res => res.json())
         .then(jsonValue => {
           this.remoteConfig_ = jsonValue;
           dev().fine(TAG, 'Remote config loaded', remoteConfigUrl);

diff --git a/extensions/amp-analytics/0.1/test/test-amp-analytics.js b/extensions/amp-analytics/0.1/test/test-amp-analytics.js
@@ -92,7 +92,11 @@ describe('amp-analytics', function() {
           } else {
             expect(init.credentials).to.undefined;
           }
-          return Promise.resolve(JSON.parse(jsonMockResponses[url]));
+          return Promise.resolve({
+            json() {
+              return Promise.resolve(JSON.parse(jsonMockResponses[url]));
+            },
+          });
         }};
       });
 

diff --git a/extensions/amp-apester-media/0.1/amp-apester-media.js b/extensions/amp-apester-media/0.1/amp-apester-media.js
@@ -145,7 +145,7 @@ class AmpApesterMedia extends AMP.BaseElement {
     const url = this.buildUrl_();
     return xhrFor(this.win).fetchJson(url, {
       requireAmpResponseSourceOrigin: false,
-    });
+    }).then(res => res.json());
   }
 
   /** @param {string} id

diff --git a/extensions/amp-apester-media/0.1/test/test-amp-apester-media.js b/extensions/amp-apester-media/0.1/test/test-amp-apester-media.js
@@ -67,7 +67,11 @@ describe('amp-apester-media', () => {
       attemptChangeSizeSpy = sandbox.spy(
           media.implementation_, 'attemptChangeHeight');
       xhrMock = sandbox.mock(xhrFor(iframe.win));
-      xhrMock.expects('fetchJson').returns(Promise.resolve(response));
+      xhrMock.expects('fetchJson').returns(Promise.resolve({
+        json() {
+          return Promise.resolve(response);
+        },
+      }));
       for (const key in attributes) {
         media.setAttribute(key, attributes[key]);
 

diff --git a/extensions/amp-app-banner/0.1/amp-app-banner.js b/extensions/amp-app-banner/0.1/amp-app-banner.js
@@ -383,7 +383,8 @@ export class AmpAndroidAppBanner extends AbstractAppBanner {
 
     return xhrFor(this.win).fetchJson(this.manifestHref_, {
       requireAmpResponseSourceOrigin: false,
-    }).then(response => this.parseManifest_(response))
+    }).then(res => res.json())
+        .then(json => this.parseManifest_(json))
         .catch(error => {
           this.hide_();
           rethrowAsync(error);

diff --git a/extensions/amp-app-banner/0.1/test/test-amp-app-banner.js b/extensions/amp-app-banner/0.1/test/test-amp-app-banner.js
@@ -112,7 +112,11 @@ describes.realWin('amp-app-banner', {amp: true}, () => {
         manifest.setAttribute('href', manifestObj.href);
         iframe.doc.head.appendChild(manifest);
         sandbox.mock(xhrFor(iframe.win)).expects('fetchJson')
-            .returns(Promise.resolve(manifestObj.content));
+          .returns(Promise.resolve({
+            json() {
+              return Promise.resolve(manifestObj.content);
+            },
+          }));
       }
 
       const banner = iframe.doc.createElement('amp-app-banner');

diff --git a/extensions/amp-auto-ads/0.1/amp-auto-ads.js b/extensions/amp-auto-ads/0.1/amp-auto-ads.js
@@ -79,6 +79,7 @@ export class AmpAutoAds extends AMP.BaseElement {
     };
     return xhrFor(this.win)
         .fetchJson(configUrl, xhrInit)
+        .then(res => res.json())
         .catch(reason => {
           user().error(TAG, 'amp-auto-ads config xhr failed: ' + reason);
           return null;

diff --git a/extensions/amp-auto-ads/0.1/test/test-amp-auto-ads.js b/extensions/amp-auto-ads/0.1/test/test-amp-auto-ads.js
@@ -135,7 +135,11 @@ describes.realWin('amp-auto-ads', {
 
     xhr = xhrFor(env.win);
     xhr.fetchJson = () => {
-      return Promise.resolve(configObj);
+      return Promise.resolve({
+        json() {
+          return Promise.resolve(configObj);
+        },
+      });
     };
     sandbox.spy(xhr, 'fetchJson');
 

diff --git a/extensions/amp-call-tracking/0.1/amp-call-tracking.js b/extensions/amp-call-tracking/0.1/amp-call-tracking.js
@@ -36,7 +36,8 @@ let cachedResponsePromises_ = {};
  */
 function fetch_(win, url) {
   if (!(url in cachedResponsePromises_)) {
-    cachedResponsePromises_[url] = xhrFor(win).fetchJson(url);
+    cachedResponsePromises_[url] = xhrFor(win).fetchJson(url)
+        .then(res => res.json());
   }
   return cachedResponsePromises_[url];
 }

diff --git a/extensions/amp-call-tracking/0.1/test/test-amp-call-tracking.js b/extensions/amp-call-tracking/0.1/test/test-amp-call-tracking.js
@@ -50,7 +50,11 @@ describe('amp-call-tracking', () => {
     xhrMock
         .expects('fetchJson')
         .withArgs(url)
-        .returns(Promise.resolve(response));
+        .returns(Promise.resolve({
+          json() {
+            return Promise.resolve(response);
+          },
+        }));
   }
 
   function expectHyperlinkToBe(callTrackingEl, href, textContent) {

diff --git a/extensions/amp-form/0.1/amp-form.js b/extensions/amp-form/0.1/amp-form.js
@@ -412,9 +412,9 @@ export class AmpForm {
         })
         .then(() => this.doXhr_())
         .then(response => this.handleXhrSubmitSuccess_(response),
-            error => this.handleXhrSubmitFailure_(
-                /** @type {!../../../src/service/xhr-impl.FetchError} */ (
-                    error)));
+            error => {
+              return this.handleXhrSubmitFailure_(/** @type {!Error} */(error));
+            });
 
     if (getMode().test) {
       this.xhrSubmitPromise_ = p;
@@ -488,19 +488,25 @@ export class AmpForm {
 
   /**
    * Transition the form the the submit error state.
-   * @param {../../../src/service/xhr-impl.FetchError} errorResponse
+   * @param {!Error} error
    * @private
    */
-  handleXhrSubmitFailure_(errorResponse) {
-    const error = (errorResponse && errorResponse.error) || errorResponse;
-    this.triggerAction_(
-        /* success */ false, errorResponse ? errorResponse.responseJson : null);
-    this.analyticsEvent_('amp-form-submit-error');
-    this.cleanupRenderedTemplate_();
-    this.setState_(FormState_.SUBMIT_ERROR);
-    this.renderTemplate_(errorResponse.responseJson || {});
-    this.maybeHandleRedirect_(errorResponse.response);
-    user().error(TAG, `Form submission failed: ${error}`);
+  handleXhrSubmitFailure_(error) {
+    let promise;
+    if (error && error.response) {
+      promise = error.response.json().catch(() => null);
+    } else {
+      promise = Promise.resolve(null);
+    }
+    return promise.then(responseJson => {
+      this.triggerAction_(/* success */ false, responseJson);
+      this.analyticsEvent_('amp-form-submit-error');
+      this.cleanupRenderedTemplate_();
+      this.setState_(FormState_.SUBMIT_ERROR);
+      this.renderTemplate_(responseJson || {});
+      this.maybeHandleRedirect_(error.response);
+      user().error(TAG, `Form submission failed: ${error}`);
+    });
   }
 
   /** @private */
@@ -549,7 +555,7 @@ export class AmpForm {
 
   /**
    * Handles response redirect throught the AMP-Redirect-To response header.
-   * @param {!../../../src/service/xhr-impl.FetchResponse} response
+   * @param {../../../src/service/xhr-impl.FetchResponse} response
    * @private
    */
   maybeHandleRedirect_(response) {