Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetching JSON content (e.g. _graph.json_) or 404 files are breaking local dev E-Tag caching mechanism #908

Closed
1 of 5 tasks
thescientist13 opened this issue Mar 5, 2022 · 1 comment · Fixed by #909 or #910
Closed
1 of 5 tasks
Assignees
Labels
bug Something isn't working CLI P0 Critical issue that should get addressed ASAP v0.24.1 v0.24.2
Milestone

Comments

@thescientist13
Copy link
Member

Type of Change

  • New Feature Request
  • Documentation / Website
  • Improvement / Suggestion
  • Bug
  • Other (please clarify below)

Summary

Looks like a case was missed as part of #629 / #760 but it seems if a request comes in with 'Content-Type: application/json' the project breaks with an error on the request.

% yarn develop
yarn run v1.22.5
$ greenwood develop
-------------------------------------------------------
Welcome to Greenwood (v0.24.0) ♻️
-------------------------------------------------------
Running Greenwood with the develop command.
Initializing project config
Initializing project workspace contexts
Generating graph of workspace files...
building from local sources...
Started local development server at localhost:1984
Unable to look up greenwood-starter-presentation using NodeJS require.resolve.
Unable to look up greenwood-starter-presentation using NodeJS require.resolve.
Unable to look up greenwood-starter-presentation using NodeJS require.resolve.
Unable to look up greenwood-starter-presentation using NodeJS require.resolve.

  TypeError: inputString.charCodeAt is not a function
      at hashString (file:///Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lib/hashing-utils.js:6:40)
      at file:///Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:141:26
      at dispatch (/Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/koa-compose/index.js:42:32)
      at file:///Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:125:11
      at async file:///Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:88:5
      at async file:///Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:51:5

Screen Shot 2022-03-05 at 2 14 55 PM

Details

Seems to be because this logic passes body right to the hashing function

const inm = ctx.headers['if-none-match'];
const etagHash = hashString(body);

But in the case of JSON, body is an Array, which means it wont have a charAt method since it is not a string. A quick test shows this simple check can solve it.

const etagHash = ctx.headers['content-type'].indexOf('application/json') >= 0
  ? hashString(JSON.stringify(body))
  : hashString(body);

Should probably add a test or figure out how to test for it.

@thescientist13 thescientist13 added bug Something isn't working P0 Critical issue that should get addressed ASAP CLI labels Mar 5, 2022
@thescientist13 thescientist13 added this to the 1.0 milestone Mar 5, 2022
@thescientist13 thescientist13 self-assigned this Mar 5, 2022
@thescientist13 thescientist13 moved this from IN PROGRESS to DONE in 7 - SSR and External Data Sources Mar 6, 2022
@thescientist13 thescientist13 changed the title fetching JSON content (e.g. _graph.json_) is breaking local dev E-Tag caching mechanism fetching JSON content (e.g. _graph.json_) or 404 files are breaking local dev E-Tag caching mechanism Mar 12, 2022
@thescientist13
Copy link
Member Author

Found another edge case, like with TypeScript sourcemaps which are currently not working, so have an empty body and should be excluded from trying to be E-Tag cached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CLI P0 Critical issue that should get addressed ASAP v0.24.1 v0.24.2
Projects
No open projects
1 participant