Fatal effect @fastify/static

GHNewbiee commented 1 year ago

Prerequisites

[X] I have written a descriptive issue title
[X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.19.2

Plugin version

4.5.0

Node.js version

20.4

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Linux Mint 21.1 Cinnamon

Description

After @fastify/caching is activated (by uncommenting the first 4 lines) the index.html loading continues and never terminates successfully. Tia

Steps to Reproduce

app.js

import Fastify from 'fastify'
// import fastifyCaching from '@fastify/caching'
import fastifyStatic from '@fastify/static'
// import abstractCache from 'abstract-cache'

const fastify = Fastify()
// const abCache = abstractCache({ useAwait: true })

// await fastify.register(fastifyCaching, {cache: abCache})
await fastify.register(fastifyStatic)

fastify.get('/json', {}, async (request, reply) => {
  reply.type('application/json').code(200)
  return { hello: 'world' }
})

fastify.get('/html', {}, async (request, reply) => {
  return reply.sendFile('index.html')
})

const start = async () => {
  try {
    await fastify.listen({port: 8000, host: 'localhost'})
  } catch (err) {
    // fastify.log.error(err)
    console.log('Found error: ', err)
    process.exit(1)
  }
}
start()

index.html

<html>
  <body>
    <b>Welcome!</b>
  </body>
</html>

Expected Behavior

fastifyCaching should not interfere with fastifyStatic so that index.html to be successfully loaded.

mcollina commented 1 year ago

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

GHNewbiee commented 1 year ago

Sorry, there is already a bug report relative to the current issue.

mcollina commented 1 year ago

Which closed as stale for inactivity and because it was missing a reproduction.

GHNewbiee commented 1 year ago

Sorry but this does not mean that there is not a bug since Feb 10, 2020. I suspect all project developers have a testing platform ready. Anyway, I have simplified the code above to the minimum.

mcollina commented 1 year ago

Sorry but this does not mean that there is not a bug since Feb 10, 2020.

The Fastify ecosystem is built by the community. Therefore a bug is fixed if somebody _volunteers to fix it (Fastify is not a company). There is no free support, and no one here will fix your bugs.

We are always happy to review pull requests.

Encapsulating the two plugins should provide a workaround:

import Fastify from 'fastify'
// import fastifyCaching from '@fastify/caching'
import fastifyStatic from '@fastify/static'
// import abstractCache from 'abstract-cache'

const fastify = Fastify()

fastify.register(async function (fastify) {
  const abCache = abstractCache({ useAwait: true })

  await fastify.register(fastifyCaching, {cache: abCache})
  fastify.get('/json', {}, async (request, reply) => {
    reply.type('application/json').code(200)
    return { hello: 'world' }
  })
})

fastify.register(async function (fastify) {

  await fastify.register(fastifyStatic)

  fastify.get('/html', {}, async (request, reply) => {
    return reply.sendFile('index.html')
  })
})

const start = async () => {
  try {
    await fastify.listen({port: 8000, host: 'localhost'})
  } catch (err) {
    // fastify.log.error(err)
    console.log('Found error: ', err)
    process.exit(1)
  }
}
start()

A pull request would be highly welcomed.

GHNewbiee commented 1 year ago

The Fastify ecosystem is built by the community. Therefore a bug is fixed if somebody _volunteers to fix it (Fastify is not a > company).

All these are already widely known and well respected!

There is no free support,

This is not a support. It's not additional feature, service, etc.

and no one here will fix your bugs.

And definitely, it is not my bug! It is a bug in one of "your" projects!

At least, what you would do in good faith is to add a conspicuous remark to both projects informing about the unpleasant interference. This will save a lot of time from the visiting users to find out what happens.

A pull request would be highly welcomed.

With all the respect, but There is no free support, and no one here will fix your bugs.

jsumners commented 1 year ago

I am unable to reproduce this with the script corrected to actually work:

import Fastify from 'fastify'
import fastifyStatic from '@fastify/static'

const fastify = Fastify()

import fastifyCaching from '@fastify/caching'
import abstractCache from 'abstract-cache'
const abCache = abstractCache({ useAwait: true })
await fastify.register(fastifyCaching, {cache: abCache})

import url from 'url'
import path from 'path'
await fastify.register(fastifyStatic, {
  // Note that we are defining the path to the static file directory
  root: path.dirname(url.fileURLToPath(import.meta.url))
})

fastify.get('/json', {}, async (request, reply) => {
  reply.type('application/json').code(200)
  return { hello: 'world' }
})

fastify.get('/html', {}, async (request, reply) => {
  return reply.sendFile('index.html')
})

const start = async () => {
  try {
    await fastify.listen({port: 8000, host: 'localhost'})
  } catch (err) {
    // fastify.log.error(err)
    console.log('Found error: ', err)
    process.exit(1)
  }
}
start()

If you can provide a reproduction that does not require any changes, feel free to reopen.

screenshot

GHNewbiee commented 1 year ago

@jsumners Thanks a lot for your time! Please watch below a screen record of the result of your code.

https://github.com/fastify/fastify-caching/assets/26875662/87d3b4a3-cc34-40d7-bc70-62f4f7957cf1

GHNewbiee commented 1 year ago

Adding reply.header('Cache-Control', 'no-store') seems to be a quick workaround:

fastify.get('/html', {}, async (request, reply) => {
  // console.log(path.dirname(url.fileURLToPath(import.meta.url)))
  reply.header('Cache-Control', 'no-store')
  return reply.sendFile('index.html')
})

I am not sure about any side effect. At least, files (not served before) are served now from localhost. A hint/remark/note might be added at both projects.

GHNewbiee commented 1 year ago

Also adding return reply.sendFile('index.html', { cacheControl: false }) seems to work as well.

fastify.get('/html', {}, async (request, reply) => {
  // console.log(path.dirname(url.fileURLToPath(import.meta.url)))
  return reply.sendFile('index.html', { cacheControl: false })
})

fastify / fastify-caching