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

Response content type is not honored #9

Closed
csarnataro opened this issue Apr 11, 2021 · 1 comment
Closed

Response content type is not honored #9

csarnataro opened this issue Apr 11, 2021 · 1 comment

Comments

@csarnataro
Copy link

Hello,
I'm building a simple JSON API with Fastify, and I'm trying to add a caching layer using your plugin. The integration of the plugin itself was pretty smooth.

Actual behaviour

The content type of the cached response is always plain/text, instead of application/json as the original, non-cached response.

Expected behaviour

The content type of the cached response should honor the content type of the initial, non-cached response.

Initial investigations:

I assume that this is happening because of this instruction

return res.code(cached.statusCode).send(cached.payload)

Probably since payload in onSend hook is always a string, and cached.payload is deserialized as a string accordingly, then Fastify's Reply.send function is using plain/text as explained here: https://www.fastify.io/docs/latest/Reply/#senddata

If this is confirmed, I don't know exactly which can be the best solution. I have a couple of solutions in mind, but I haven't spend much time on it, yet.

  1. First option (simpler, but less flexible): add an additional option in input to fastify-response-caching, so that the content type can be set based on this option. This option can be a string or a function (or a Promise maybe, if we want to parse the payload to check if it's a valid JSON?).
    E.g.
fastify.register(fastifyResponseCaching, {ttl: 5000, contentType: 'application/json; charset=utf-8' })

or

fastify.register(fastifyResponseCaching, {
  ttl: 5000, 
  contentType: (payload) => getContentTypeBasedOnPayload(payload)
})
  1. Second option: being able to determine the content type based on the payload in the onSend hook, so that we can set the right content type in cache.set .

Any thoughts on this?

Steps to reproduce:

  1. git clone https://github.com/csarnataro/reproduce-fastify-response-caching-content-type.git

  2. npm run start

  3. open you browser to http://localhost:3000/

  4. in the network inspector, check that the first uncached response has:
    content-type -> application/json; charset=utf-8

  5. refresh the page at http://localhost:3000/

  6. check that the second cached response has:
    content-type -> text/plain; charset=utf-8

@TheTimmaeh
Copy link
Contributor

Since this bugged me, here's a fix #10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants