Skip to content

Conversation

AgentHagu
Copy link
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
Partially fixes #2717

Adds an error handler to explicitly send the 404.html upon error 404. It removes the previous check for err.status === 404 and handles the error in a newly added middleware.

Anything you'd like to highlight/discuss:
This PR only partially fixes #2717. It ensures that when serving the site locally, any invalid pages without a corresponding file will properly respond with the 404.html file. It does not fix the current rendering issue with the default 404.html file causing it to be blank. That issue is related to #2716.

Testing instructions:

  1. Locally serve a MarkBind site using markbind serve
  2. Try accessing an invalid page, like http://127.0.0.1:8080/test
  3. Verify that the 404.html file is shown as the response (To avoid Centre-align feature is not working #2716, this requires adjusting 404.html such as removing the use of center-align)

Proposed commit message: (wrap lines at 72 characters)
Fix issue with 404 page not working locally


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

Copy link

codecov bot commented Jun 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.84%. Comparing base (1014aad) to head (fed4f74).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2724   +/-   ##
=======================================
  Coverage   52.84%   52.84%           
=======================================
  Files         130      130           
  Lines        7162     7162           
  Branches     1594     1501   -93     
=======================================
  Hits         3785     3785           
  Misses       3072     3072           
  Partials      305      305           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AgentHagu
Copy link
Contributor Author

Personally, I'm not sure if this is the best fix because I'm not too familiar with the other middleware, such as serveIndex. Also not too sure if this is a new issue after v6 or an existing issue in v5 that we didn't notice previously. My own testing seems to show that the bug also exists in v5.

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 this pull request may close these issues.

404 page not working
1 participant