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

Remove database_file_handler; give precendence to router error messages #1326

Merged
merged 4 commits into from
Mar 22, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 21, 2021

The database handler was unused; the router already serves the favicon and it
doesn't do anything else. Note that 'database handler' is different
from 'files served from storage', the router already handles those.

This also fixes the long standing bug that all 404 errors are either
'crate not found' or 'resource not found'.

Closes #55. This builds on #1324.

When serving 'essential files', we can either serve the global one,
created when building `empty_library`, or the local one, created when
building the local crate. Currently we default to the global one, but
this causes issues when the file should never have been global in the
first place (such as recently for `crates.js`: see
rust-lang#1313).

This gives precedence to the local file so that the bug will be fixed
when rustdoc fixes it, even if we forget to update
`ESSENTIAL_FILES_UNVERSIONED`.
…ages

The database handler was unused; the router already serves the favicon and it
doesn't do anything else. Note that 'database handler' is different
from 'files served from storage', the router already handles those.

This also fixes the long standing bug that all 404 errors are either
'crate not found' or 'resource not found'.
@jyn514 jyn514 added S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed A-backend Area: Webserver backend labels Mar 21, 2021
@Nemo157
Copy link
Member

Nemo157 commented Mar 21, 2021

According to the database success route metric I see 26 requests over the last hour from the database handler. I was really hoping when I added it that it would just display 0 and we could trivially get rid of it, but I think there needs to be some more investigation into what requests these are that it is serving.

@jyn514
Copy link
Member Author

jyn514 commented Mar 21, 2021

@Nemo157 it's serving pages like /rustdoc/regex/1.4.3/regex/index.html. I don't think we need to keep them around - none of the redirects work and you can get the pages just as easily by removing /rustdoc/ at the front.

@jyn514
Copy link
Member Author

jyn514 commented Mar 21, 2021

It also lets you have programmatic access to the build-logs and sources (e.g. https://docs.rs/build-logs/334832/x86_64-unknown-linux-gnu.txt and https://docs.rs/sources/regex/1.4.5/CHANGELOG.md), but I don't think we should keep it around just for that. If we think build logs are useful they should be part of the router, not a completely separate handler. Serving sources directly seems like a misfeature, people should use crates.io tarballs for that.

@Nemo157
Copy link
Member

Nemo157 commented Mar 21, 2021

But how are people discovering those URLs and going to them in production? Maybe the easiest way to see if removing this breaks someone's workflow is just to merge it and see who complains 😁

@jyn514 jyn514 added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Mar 21, 2021
@jyn514 jyn514 merged commit eb8ed3d into rust-lang:master Mar 22, 2021
@jyn514 jyn514 mentioned this pull request Apr 14, 2021
@jyn514 jyn514 deleted the delete-database-handler branch April 20, 2021 03:41
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend Area: Webserver backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"The requested crate does not exist" message if a version does not exist
3 participants