zachleat / speedlify

Benchmark the web performance and accessibility of sites over time.
https://www.speedlify.dev/
MIT License
922 stars 168 forks source link

URLs without trailing slash breaks build #23

Closed budparr closed 3 years ago

budparr commented 4 years ago

I've created a test instance of Speedify here: https://github.com/budparr/speedlify

Problem

When building the site from generated results, the following (in _data/sites) breaks the build

urls: ["https://www.11ty.dev/", "https://www.11ty.dev/docs/getting-started"],
Error Detail ``` Uncaught exception: (more in DEBUG output) > The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received type boolean (false) `TypeError` was thrown: TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received type boolean (false) at writeFile (fs.js:1385:5) at go$writeFile (/Users/budparr/Code/prs/speedlify/node_modules/graceful-fs/graceful-fs.js:139:14) at Object.writeFile (/Users/budparr/Code/prs/speedlify/node_modules/graceful-fs/graceful-fs.js:136:12) at /Users/budparr/Code/prs/speedlify/node_modules/fs-extra/lib/output/index.js:23:10 at /Users/budparr/Code/prs/speedlify/node_modules/fs-extra/lib/mkdirs/mkdirs.js:56:16 at callback (/Users/budparr/Code/prs/speedlify/node_modules/graceful-fs/polyfills.js:295:20) at FSReqCallback.oncomplete (fs.js:177:5) ```

Immediate Fix

Changing the URLs to have a trailing slash creates no error:

urls: ["https://www.11ty.dev/", "https://www.11ty.dev/docs/getting-started/"],

note the change is adding a trailing slash to the end of the URL

Expected Behavior

I would expect that Speedlify would not care if a user put trailing slashes in their URL

Potential Solution

I suspect this is because of a site hash mismatch from api.11ty.js, but I haven't figured out how to fix it yet, so reporting in case this is an easy fix for someone else.

zachleat commented 4 years ago

I don’t think this is due to the trailing slash. Your repo test case works fine over here:

image image

What version of Node are you using? It’s failing to write a one of the JSON results files. Is the directory writable?

budparr commented 4 years ago

Screen Shot 2020-07-26 at 11 44 49 AM

Note the results directory, which seem to come from the hashed values of the URLs are:

a1e5af56 for https://www.11ty.dev/ f75b90c3 for https://www.11ty.dev/docs/getting-started

Yet, if I log out data.site.hash in api.11ty.js I get

hash a1e5af56
hash 4075b72c

so, there's a mismatch. My runt-tests is outputting f75b90c3 and api.11ty.js is using 4075b72c

To confirm, logging out result.url and id (the hash) in run-tests.js I get

result.url https://www.11ty.dev/
id a1e5af56

result.url https://www.11ty.dev/docs/getting-started/
id f75b90c3

Note, the result.url in run-tests.js has the trailing slash, even though I didn't enter it. That's where the mismatch comes from and why the file is not being written.

Sorry if I'm not being helpful enough here. I'll have another look at it later.

budparr commented 4 years ago

Changing let id = shortHash(result.url); in run-tests.js#L113 to let id = shortHash(result.requestedUrl); fixes the issue.

result.url {
  url: 'https://www.11ty.dev/docs/getting-started/',
  requestedUrl: 'https://www.11ty.dev/docs/getting-started',
...

I'm happy to submit a PR if that makes sense to you.

zachleat commented 3 years ago

I believe this was solved on the other end (not in the hashing in run-tests.js specifically but in the output templates), specifically when the code was added to de-dupe redirects

Can you try again?

image

zachleat commented 3 years ago

Closing for now! Feel free to leave additional feedback