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

[RTD] New readthedocs-addons.js not proxied properly, results in 404 #537

Open
msbt opened this issue Oct 14, 2024 · 5 comments
Open

[RTD] New readthedocs-addons.js not proxied properly, results in 404 #537

msbt opened this issue Oct 14, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@msbt
Copy link
Collaborator

msbt commented Oct 14, 2024

As mentioned the other day (in #536), RTD introduced new addons which broke some of our custom integrations. We fixed most of it, but the addons themselves aren't being loaded because apparently they're ignoring the proxied_static_path directive from our sphinx template, which results in a 404 and subsequently in the loss of some of the functionality (e.g. version-warning on unstable/older releases, PR's and such - only on proxied URLs of course).

So instead of loading the file from
https://cratedb.com/docs/crate/reference/_/static/javascript/readthedocs-addons.js
it's looking in the domain root
https://cratedb.com/_/static/javascript/readthedocs-addons.js

In our theme we tried to override the path via https://github.com/crate/crate-docs-theme/blob/main/src/crate/theme/rtd/conf/__init__.py#L292-L309 which used to work, but not anymore.

@amotl
Copy link
Member

amotl commented Oct 15, 2024

Thanks for reporting and providing a good analysis, where this is coming from, and where to look for a fix.

As long as it is not super urgent 1, I am postponing to resolve it in an upcoming long winter's night, maybe after doing one more hands-on analysis session together with you, as this often worked very well to tame the Sphinx.

Footnotes

  1. Please let me know if the fact that a 404 error is raised, does any harm to any kind of web reputation machine, where its spider doesn't like 404's too much?

@amotl amotl added the bug Something isn't working label Oct 15, 2024
@amotl amotl changed the title New RTD readthedocs-addons.js not proxied properly, results in 404 Documentation: New RTD readthedocs-addons.js not proxied properly, results in 404 Oct 15, 2024
@msbt
Copy link
Collaborator Author

msbt commented Oct 17, 2024

@amotl hard to say how big that impact really is, I just saw a bunch (2k) of 404s in SEO tools which has an overall impact on our scoring, but unsure if Google or any other search engine is handing out penalties for broken js. Either way, I would be keen to tackle it anytime soon, but lets sync on the other platform to figure out what to do.

@msbt
Copy link
Collaborator Author

msbt commented Oct 31, 2024

@amotl I did a bit of researching, this is what I found:

Somewhat explained here https://github.com/readthedocs/readthedocs.org/blob/main/readthedocs/proxito/README.rst#custom-path-prefixes and maybe here https://dev.readthedocs.io/en/latest/design/better-doc-urls-handling.html

I'm not a 100% sure if the custom path also affects the /_/ issue, but it would be a start, since we could "just" use our url_path variable as prefix for each repo!

@amotl
Copy link
Member

amotl commented Oct 31, 2024

Thanks for the research. If you see a chance there is a chance for fixing inside, let's try to apply it together?

@msbt
Copy link
Collaborator Author

msbt commented Dec 2, 2024

@amotl did a bit more research: Can we maybe override the prefix from these settings or is that only a description of how rtd is handling it?

It seems that if we're able to use the prefix = "/" variable, that would solve our custom canonical rewrite (except the host part) and also the path of the addons, as long as they also forward /_, which I'm not a 100% sure, maybe you can quickly browse the following links since you have a better understanding of what runs in the background 🙏

1 "Proxito will process all documentation requests from a single docs serve view, excluding /_ URLs."
2 "To change the path prefix of a project, define the prefix in the custom_prefix attribute of the project itself. For a translation, change the main language project custom_prefix attribute."
3 "Maybe... The main use case here would be prefixing I think, as well as users proxying to us. We have hit issues in the past where we assume that we own the entire domain's hosting (docs.example.org/_/), but when there's a prefix, we likely want to serve it at {prefix}/_/, in case we aren't hosting the root domain in some configuration."

Footnotes

  1. https://docs.readthedocs.com/dev/stable/design/better-doc-urls-handling.html#look-up-process

  2. https://github.com/readthedocs/readthedocs.org/tree/main/readthedocs/proxito#where-to-define-the-custom-path-prefix

  3. https://github.com/readthedocs/readthedocs.org/pull/9425#discussion_r918407407

@amotl amotl changed the title Documentation: New RTD readthedocs-addons.js not proxied properly, results in 404 [RTD] New readthedocs-addons.js not proxied properly, results in 404 Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants