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

When the file is not found, Fastify's 404 handler is not triggered #218

Closed
2 tasks done
BlackGlory opened this issue Jul 8, 2021 · 0 comments · Fixed by #225
Closed
2 tasks done

When the file is not found, Fastify's 404 handler is not triggered #218

BlackGlory opened this issue Jul 8, 2021 · 0 comments · Fixed by #225

Comments

@BlackGlory
Copy link
Contributor

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure it has not already been reported

Fastify version

3.18.1

Plugin version

4.2.2

Node.js version

v14.17.1

Operating system

Linux

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

Ubuntu 20.04.2 LTS

Description

As described in the title, when the file is not found, Fastify's 404 handler should handle the error.

The reproduction example is from my project, which adds cache headers to 404 responses.
The cache header should be added successfully, but it fails in the integration test.

After looking at the source code, I found that fastify-static did not handle this case correctly, and the test passed with the following changes:
https://github.com/BlackGlory/fastify-static/commit/37cd6b10f4608d0aec36fcb4f18e127c13de5a81

Steps to Reproduce

git clone https://github.com/BlackGlory/static.git
cd static
git checkout 429479a
yarn install

Uncomment the line in __tests__/services/index.spec.ts.

yarn test
 FAIL  __tests__/services/files/index.spec.ts
  ● files › GET /files/:location › file does not exist › 404

    expect(received).toBe(expected) // Object.is equality

    Expected: "private, no-cache, no-store, max-age=0, must-revalidate"
    Received: null

      52 |         expect(res.status).toBe(404)
      53 |         // very strange, it should pass the test.
    > 54 |         expect(res.headers.get('Cache-Control')).toBe(NOT_FOUND_CACHE_CONTROL())
         |                                                  ^
      55 |       })
      56 |     })
      57 |

      at Object.<anonymous> (__tests__/services/files/index.spec.ts:54:50)

Expected Behavior

Handling Errors

If an error occurs while trying to send a file, the error will be passed to Fastify's error handler. You can set a custom error handler with fastify.setErrorHandler().

GormanFletcher pushed a commit to GormanFletcher/fastify-static that referenced this issue Aug 4, 2021
GormanFletcher pushed a commit to GormanFletcher/fastify-static that referenced this issue Aug 4, 2021
@Eomm Eomm closed this as completed in #225 Aug 5, 2021
Eomm pushed a commit that referenced this issue Aug 5, 2021
* fix: handle NotFoundError

* fix: Add regression test to call 404 handler when send ignores a dotfile (#218)

Co-authored-by: BlackGlory <woshenmedoubuzhidao@blackglory.me>
Co-authored-by: Gorman Fletcher <git@gormanfletcher.com>
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

Successfully merging a pull request may close this issue.

1 participant