Skip to content

Commit

Permalink
Shorten URL in error messages (#32818)
Browse files Browse the repository at this point in the history
Currently the full URL is too long for Gmail since the URL includes
`__amp_source_origin`, which is the full data URL for the AMP document
itself.

This follows the eliding pattern for the non-SSR path in xhr-impl:

https://github.com/ampproject/amphtml/blob/a0f71dfb0381e236d6c6be287549748838731467/src/service/xhr-impl.js#L11
  • Loading branch information
zhangsu committed Feb 23, 2021
1 parent 93b5760 commit 7624abd
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 7 deletions.
22 changes: 17 additions & 5 deletions extensions/amp-list/0.1/amp-list.js
Expand Up @@ -852,24 +852,25 @@ export class AmpList extends AMP.BaseElement {
userAssert(
response,
'Failed fetching JSON data: XHR Failed fetching ' +
`(${request.xhrUrl}): received no response.`
`(${this.buildElidedUrl_(request)}): received no response.`
);
const init = response['init'];
if (init) {
const status = init['status'];
if (status >= 300) {
/** HTTP status codes of 300+ mean redirects and errors. */
throw user().createError(
`Failed fetching JSON data (${request.xhrUrl}): HTTP error`,
`Failed fetching JSON data (${this.buildElidedUrl_(request)})` +
': HTTP error',
status
);
}
}
userAssert(
typeof response['html'] === 'string',
'Failed fetching JSON data: XHR Failed fetching ' +
`(${request.xhrUrl}): Expected response with format ` +
'html: <string>}. Received: ',
`(${this.buildElidedUrl_(request)}): Expected response with ` +
'format {html: <string>}. Received: ',
response
);
request.fetchOpt.responseType = 'application/json';
Expand All @@ -878,7 +879,7 @@ export class AmpList extends AMP.BaseElement {
(error) => {
throw user().createError(
'Failed fetching JSON data: XHR Failed fetching ' +
`(${request.xhrUrl})`,
`(${this.buildElidedUrl_(request)})`,
error
);
}
Expand All @@ -892,6 +893,17 @@ export class AmpList extends AMP.BaseElement {
});
}

/**
* Builds an elided, shortened URL suitable for display in error messages from
* the given request.
* @param {!FetchRequestDef} request
* @return {string}
*/
buildElidedUrl_(request) {
const url = Services.urlForDoc(this.element).parse(request.xhrUrl);
return `${url.origin}/...`;
}

/**
* Schedules a fetch result to be rendered in the near future.
* @param {!Array|!JsonObject|undefined} data
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-list/0.1/test/test-amp-list.js
Expand Up @@ -829,7 +829,7 @@ describes.repeated(
return expect(
list.layoutCallback()
).to.eventually.be.rejectedWith(
/XHR Failed fetching \(https:\/\/data.com.+?\): error/
/XHR Failed fetching \(https:\/\/data.com\/\.\.\.\): error/
);
});

Expand All @@ -853,7 +853,7 @@ describes.repeated(
return expect(
list.layoutCallback()
).to.eventually.be.rejectedWith(
/fetching JSON data \(https:\/\/data.com.+?\): HTTP error 400/
/fetching JSON data \(https:\/\/data.com\/\.\.\.\): HTTP error 400/
);
});

Expand Down

0 comments on commit 7624abd

Please sign in to comment.