Pull request preview images are missing (links are broken)

[rjpadula]

Images in the generated preview are not being displayed. See Figure 1 in PR 167 for an example.

For each image, the console shows a 403:

[Error] Failed to load resource: the server responded with a status of 403 (Forbidden) (VerifiableCredentialsUserRoles.png, line 0)

Attempting to access the image directly via https://pr-preview.s3.amazonaws.com/w3c/vc-use-cases/pull/VerifiableCredentialsUserRoles.png yields an AWS permission issue:

<Error> <Code>AccessDenied</Code> <Message>Access Denied</Message> <RequestId>XTS5BG6Q7WNT5Q1X</RequestId> <HostId> qQJ6FFhsXamAa3dsoYpjayBX8J1V/GylgLjYkeMBAlVf8VCGrnem/LbxwLoIl5b7J4lhkKGnSNi4dJwDh0zQErXX34xf3Asa </HostId> </Error>

The AWS message does not mean that the file exists.

Note that the image's URL does not contain the pull request number (167 in this case), while the HTML file's URL does.

This issue is likely with w3c/spec-prod or one of the actions it calls, but this is a solid reproduction of it here.

[jandrieu]

@iherman Can you help us track down who or where to escalate this to?

@rjpadula was able to find that the build-and-upload works for publishing from main, but not for PRs. It's not clear if the files are getting skipped in the PR preview or if they are getting uploaded, but AWS has incorrect permissions set for the bucket.

If we had access to AWS, we could try to debug further, but we're at a bit of a bottleneck trying to reverse engineer the process.

[iherman]

Unfortunately, that is a limitation of the preview mechanism: it creates a local copy for the software to run through respec and generate a viewable HTML, but the copying mechanism is not sophisticated enough to follow up on such external links (in contrast to echidna, which does that for the final publication). The only way to get a full preview is to clone the repo (or the relevant branch) and view at it locally (preferably through localhost, to avoid limitation withe file: references).

It is a pain in the neck, and a possible improvement request to the preview software, but we have to live with it. Sorry...

[rjpadula]

Do you know which action or part of the preview software is actually performing the upload into AWS ?
The index.html file gets renamed into PR#.html by something.
I'd be happy to dig into that

[iherman]

Preview is done via https://github.com/tobie/pr-preview as a github app. I do not know the details.

[jandrieu]

@iherman you said

The only way to get a full preview is to clone the repo (or the relevant branch) and view at it locally (preferably through localhost, to avoid limitation with file: references).

I don't believe this is correct. Github pages works fine, even when echidna was failing. So, for example, while @KDean-Dolphin and I were debugging issues with Echidna for vc-use-cases, the github.io pages rendered fine.

Also, I'm not sure it matters, but it's not file: resources but local urls like '/images/image1.png'. They work fine in github pages, so the workflow (before echidna) seems to work well.

I've bumped an older issue over at pr-preview tobie/pr-preview#144 in hopes of engaging @tobie or anyone who is capable of debugging that S3 transaction (we don't have access to further debug that part of PR-Preview).

[tobie]

I'm not sure whether PR preview has any code to support images. Have you dug into the src? (It's been a while since I looked into it seriously.)

[jandrieu]

I'm not sure whether PR preview has any code to support images. Have you dug into the src? (It's been a while since I looked into it seriously.)

@rjpadula has been working through the src and we are a bit stymied because we can't debug what is going to S3.

Is there a reason the entire file set from the PR branch isn't being copied? The upload capability seemed to be agnostic to file type, but apparently we aren't reading the right code.

If you could point to the code that does the actual transfer, that might help.

[tobie]

It only uploads the spec itself and possibly the includes, iirc. There's no full copy of the repo going on.

[jandrieu]

Right. So where in the code do we fix that? And are you open to that fix?

For PRs to be renderable, you really need all of the assets in the published branch.

[tobie]

I believe taking inspiration from https://github.com/tobie/pr-preview/blob/master/lib/scan-includes.js and where that gets called is your best starting point.

[jandrieu]

And you're open to a PR with that fix?

[tobie]

If it's reasonable and doesn't require too much review time, absolutely.

[rjpadula]

Fixing this looks to be beyond our resources at this time

See tobie/pr-preview#144 (comment)