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

Do not use invalid cache #963

Closed
2 tasks done
yovanoc opened this issue Dec 4, 2019 · 19 comments
Closed
2 tasks done

Do not use invalid cache #963

yovanoc opened this issue Dec 4, 2019 · 19 comments
Assignees
Labels
bug Something does not work as it should external The issue related to an external project

Comments

@yovanoc
Copy link

yovanoc commented Dec 4, 2019

Describe the bug

  • Node.js version: 13.3
  • OS & version: MacOS Catalina

Actual behavior

I get GotError: Unexpected token in JSON at position 0 in ... when I add responseType: "json" .
But this happens 2 times out of 5 .. and If I remove the responseType and log the body, all is good..

PS: right before this error there is this warning every time: (node:34140) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

Expected behavior

Should be able to parse JSON, I think got tries to parse before getting full response or something like that

Code to reproduce

const body = await got
    .get(link, {
      cache,
      retry: 4
    })
    .json();

or

const { body } = await got
    .get(link, {
      cache,
      retry: 4,
      responseType: 'json'
    });

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@yovanoc yovanoc changed the title JSONResponses JSON Responses Dec 4, 2019
@szmarczak
Copy link
Collaborator

What's the link? If it's a custom server, please attach some code. What's the cache options?

@yovanoc
Copy link
Author

yovanoc commented Dec 4, 2019

Can we exchange by email for links? It's a private project?

@szmarczak
Copy link
Collaborator

szmarczak commented Dec 4, 2019

That won't be neccessary I think. If you open the link in your browser, what's the first character of the body?

What's the cache options?

Also if you provided the full stack trace, that'd be awesome.

@yovanoc
Copy link
Author

yovanoc commented Dec 4, 2019

The first character is the {.

Here's the cache:

const cache = new KeyvFile({
  filename: join(appDataPath, "cache.json"), // the file path to store the data
  expiredCheckDelay: 10 * 24 * 3600 * 1000, // ms, check and remove expired data in each ms
  writeDelay: 120, // ms, batch write to disk in a specific duration, enhance write performance.
  encode: JSON.stringify, // serialize function
  decode: JSON.parse // deserialize function
});

And I will try to provide you a bigger stacktrace, thanks

@szmarczak
Copy link
Collaborator

Thanks, I'll now investigate.

@szmarczak
Copy link
Collaborator

right before this error there is this warning every time

This is actually weird as Got never does Buffer(...). It does Buffer.from(...)...

@szmarczak
Copy link
Collaborator

If you can provide HTTP headers, that'd awesome as well (you may leave out some of them if they are sensitive).

@yovanoc
Copy link
Author

yovanoc commented Dec 4, 2019

Response headers? Because I don't pass any for the request

@szmarczak
Copy link
Collaborator

yup

@yovanoc
Copy link
Author

yovanoc commented Dec 4, 2019

{
  'content-type': 'application/json',
  'last-modified': 'Wed, 27 Nov 2019 01:56:55 GMT',
  'content-encoding': 'gzip',
  vary: 'Accept-Encoding',
  age: '44979'
}

@yovanoc
Copy link
Author

yovanoc commented Dec 5, 2019

@szmarczak I've done more tests. If I simply remove the cache, all is working perfectly. AND I've tried to do a setInterval to request once a second the .json (with the cache) and 1/2 times the data is completely invalid (weird ascii characters), and give the JSON.parse error.
So I think now the cache must have an issue

@szmarczak
Copy link
Collaborator

Could you provide the full error stacktrace?

@yovanoc
Copy link
Author

yovanoc commented Dec 5, 2019

SyntaxError: Unexpected token  in JSON at position 0
    at JSON.parse (<anonymous>)
    at parseBody (/Users/devchris/Documents/code/nodejs/project/node_modules/got/dist/source/as-promise.js:12:47)
    at /Users/devchris/Documents/code/nodejs/project/node_modules/got/dist/source/as-promise.js:130:47
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

How can I get more? Isn't good for you? And I checked: 1/2 times the key is removed in the cache file, so the next time he made the request, it's working, then error, and he removes, etc..

@szmarczak
Copy link
Collaborator

Unfortunately I cannot reproduce this one. If you want to you can email me here:

sz.marczak@gmail.com

I'll be more than happy to help.

@szmarczak
Copy link
Collaborator

szmarczak commented Dec 5, 2019

It seems that it's a zero-width space character: JSON.parse('​') gives the same result. Could you try .text() instead of .json() and log these bodies? Are they the same?

@yovanoc
Copy link
Author

yovanoc commented Dec 5, 2019

Just sent to you an email, thanks

@szmarczak
Copy link
Collaborator

Replied back

@szmarczak szmarczak changed the title JSON Responses Cached headers are overwritten even though the response has not been modified Dec 8, 2019
@szmarczak
Copy link
Collaborator

Culprit: kornelski/http-cache-semantics#26

@szmarczak szmarczak added bug Something does not work as it should external The issue related to an external project labels Dec 9, 2019
@szmarczak szmarczak self-assigned this Dec 9, 2019
@szmarczak szmarczak changed the title Cached headers are overwritten even though the response has not been modified Do not use invalid cache Dec 23, 2019
@szmarczak
Copy link
Collaborator

http-cache-semantics works properly. The second response doesn't include any validation header so headers shouldn't be merged.

Instead, it should make a new request or return the new response without merging anything.

I'll move this into #875

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should external The issue related to an external project
Projects
None yet
Development

No branches or pull requests

2 participants