Skip to content

Improve 404 page for missing rustdoc resources - #3416

Open
kivancgnlp wants to merge 9 commits into
rust-lang:mainfrom
kivancgnlp:fix/contextual-rustdoc-404
Open

Improve 404 page for missing rustdoc resources#3416
kivancgnlp wants to merge 9 commits into
rust-lang:mainfrom
kivancgnlp:fix/contextual-rustdoc-404

Conversation

@kivancgnlp

@kivancgnlp kivancgnlp commented Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

Improves the error page shown when a requested rustdoc resource does not exist in an otherwise valid crate version.

Instead of displaying only a generic 404 page, the response now includes links to:

  • The documentation root for the requested version
  • The crate page containing all available versions

Fixes #2568.

Changes

  • Add a contextual ResourceNotFoundInVersion error variant
  • Preserve /latest/ and pinned-version URLs when constructing recovery links
  • Render optional recovery links on the HTML error page
  • Include equivalent links in JSON error responses
  • Add tests for latest, pinned-version, JSON, and generic 404 behavior
  • Keep existing no-cache behavior for error responses

Non-goals

This PR intentionally does not:

  • Redirect missing pages automatically
  • Search previous versions for the missing resource
  • Add search integration

These can be considered separately if desired.

Testing

  • Project compiles locally
  • Existing direct AxumErrorPage initializers updated
  • Full test suite pending GitHub Actions

@kivancgnlp
fix: improve missing rustdoc page errors
f3b1e4e
@kivancgnlp
Initialize empty recovery in about 404 response
f55bb6f 
Include an empty `recovery` Vec in the about handler's not-found page response. This ensures the response struct has the newly required `recovery` field and prevents missing-field errors after the page/template was extended.
@kivancgnlp
Update about.rs
ff88e21
@github-actions github-actions Bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Jul 23, 2026
@kivancgnlp
Update about.rs
4099f20
@kivancgnlp
Only include recovery links when present
5a14429 
Refactor JSON error response construction to avoid emitting an empty "links" array. The code now builds a base body with title and message and only inserts the "links" field if recovery links are non-empty. Also minor cleanup: clarified the NoResults comment and added braces around the Redirect match arm.
@kivancgnlp
Simplify Redirect match arm formatting
06ada29 
Refactor the match arm for AxumNope::Redirect in crates/bin/docs_rs_web/src/error.rs to a single-line expression calling redirect_with_policy(target, cache_policy). This is a formatting/clarity change only and does not alter behavior.
@kivancgnlp
kivancgnlp marked this pull request as ready for review July 23, 2026 13:22
@kivancgnlp
kivancgnlp requested a review from a team as a code owner July 23, 2026 13:22
syphar
syphar requested changes Jul 24, 2026
View reviewed changes

@syphar syphar left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for the work!

only two things, looking at the actual page:

  • the error message wording can be improved
  • the styling of the links too
Image

View changes since this review

@kivancgnlp

kivancgnlp commented Jul 24, 2026

Copy link
Copy Markdown
Author

Thanks @syphar. Agreed.
I’ll simplify the wording and present the recovery links as clearer actions rather than a plain list.

@kivancgnlp
kivancgnlp requested a review from syphar July 24, 2026 12:46
@syphar

syphar commented Jul 25, 2026

Copy link
Copy Markdown
Member

Thanks @syphar. Agreed. I’ll simplify the wording and present the recovery links as clearer actions rather than a plain list.

before I spend more time reviewing this:

  • please remove all claude comments / author from the commit (in the commit message, and the added author).
  • and most importantly: please confirm that this time you actually manually tested the feature, and believe it's ready for production.

I'm ok with people using AI for their contribution, if the author also reviewed & tested everything.

@kivancgnlp
improve 404 wording and style recovery links as buttons
2137e12 
Address review feedback on rust-lang#2568: clarify the contextual missing-page
message and render the recovery links as pure-css action buttons instead
of a plain bullet list. JSON error body structure is unchanged.
@kivancgnlp
kivancgnlp force-pushed the fix/contextual-rustdoc-404 branch from 459832c to 2137e12 Compare July 25, 2026 10:03
@kivancgnlp
Center contextual 404 recovery actions
db1e00b 
The error page title lives in a `div.container`, which is only centered on
the page when the body sets the `centered` class. The error page doesn't, so
the title was centered inside a box pinned to the left edge while the message
and recovery links below centered against the full page width.

Scope the container centering to the error page with an `error-header` class
so the title lines up with the text below it, center the recovery links, and
give them a small top gap so they read as part of the message rather than
running into it.
@kivancgnlp

kivancgnlp commented Jul 25, 2026

Copy link
Copy Markdown
Author

I tested the latest branch locally by importing itoa 1.0.18 and opening both contextual 404 routes:

/itoa/latest/itoa/removed_module/index.html

/itoa/1.0.18/itoa/removed_module/index.html

I verified the 404 title and message, responsive button layout, and both recovery-link destinations. I also adjusted the title alignment and spacing after checking the rendered page.
Screenshot 2026-07-25 at 16 59 02

@GuillaumeGomez

GuillaumeGomez commented Jul 25, 2026

Copy link
Copy Markdown
Member

Please add a GUI test then. :)

@kivancgnlp

kivancgnlp commented Jul 25, 2026

Copy link
Copy Markdown
Author

Thanks for the suggestion. I added a GUI test in gui-tests/404.goml covering the contextual 404 title, message, and both recovery links using the existing sysinfo fixture. The GUI test passes locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@syphar syphar Awaiting requested review from syphar

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve behavior after publishing a new version causes /latest/ to lose children

3 participants

@kivancgnlp @syphar @GuillaumeGomez