🐛 Don't require ampCors for amp-document-recommendations requests

[peterjosling]

Fixes issue described in #12945 (comment)

[peterjosling]

@aghassemi

[aghassemi]

@peterbaran Could you elaborate in the description of this PR what part of #9423 is being addressed?

[peterjosling]

Oops, linked to the wrong issue. Updated with a link to the specific comment (in the right issue).

[aghassemi]

@peterjosling On second thought, I think CORS is needed. When in AMP viewer the fetch would originate from a non-origin domain, if they don't setup CORS from the beginning, infinite scroll will fail from AMP cache. I do not recommend merging this PR.

[aghassemi]

@peterjosling @emarchiori We also need to make sure the fetched AMP page is valid AMP. Maybe we can:
1- When served from origin (not served from cache): no CORS required, if invalid, doesn't matter.
2- When served from cache: we fetch the AMP page from AMP cache instead (there are APIs to translate AMP Url to AMP Cache Url). if not in cache, it is a failure.

[aghassemi]

The TODO you have in the code covers the comment above, so LGTM on the PR.

[peterjosling]

What API is there for getting cache URLs in the runtime?

[aghassemi]

@honeybadgerdontcare What's the best way to get cache URL of an AMP Url in the runtime? (Context: imagine publishers returning Urls to their AMP pages, we want to make sure they are valid and then ideally inject the SSRed/cached version to the existing document).

[honeybadgerdontcare]

@aghassemi I'm not aware of a function in the Runtime that does this. It is provided as an AMP URL API service and the scheme is fairly well defined that it might be possible to add it to the Runtime.

[aghassemi]

Thanks @honeybadgerdontcare @peterjosling Looks like we can just do this from this extension without adding a core service to runtime. Looks simple enough to do with fetch calls.

[peterjosling]

Using the API service would presumably require the publisher to create + specify their own API key for it, unless the rate limit is disabled from *.cdn.ampproject.org.

Thanks for the link to the scheme! Only issue with transforming it locally is that it looks like there's no code in the framework for doing punycode encoding...

[aghassemi]

I don't see requirements that Urls need to be converted to punny-code before they are passed as part of the request. I really hope the Url service works with IDL domain names without a need for clients to convert first. (If that's not the case, we can fix the service)

Have to defer to @honeybadgerdontcare for rate limit.

[honeybadgerdontcare]

It is rate limited. I'll follow up over email with you.