[go: up one dir, main page]

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

Nginx / Lua based approach to Open Graph tags #422

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

robobenklein
Copy link
Contributor
@robobenklein robobenklein commented Jul 10, 2021

This is an alternative option for #416 which I believe has the following benefits compared to #421

  • Does not require any additional processes, containers, or listening ports
  • Much lower memory & cpu footprint vs an additional python server ( =handling more traffic)
  • Smaller container size vs adding another Python webserver container
  • Lua requests to the backend run in Nginx's non-blocking event loop

There are some issues with this implementation which I want to fix first though:

  • The openresty image is much larger than the bare nginx image (101MB vs 22MB) so I would like to just add on the Lua module to the Nginx image ourselves (22+??MB) (although there are some additional complications with that process which I am still sorting out, like runtime Lua optimizations being missing in the manual install on nginx)
  • Requests to the backend API for metadata should be done in parallel (or cache the ones like /api/info)

NOTE: this is still a work in progress (draft PR)

Things still TODO:

  • Can all the page to API requests be done simply by adding /api to the front of them? (/post/1 -> /api/post/1 and /tag/name -> /api/tag/name) No, some page links might include information afterwards, like /post/1/query=cooltag
    • If so, should we depend on this in the future? (I think it would reduce maintenance effort later on if it does change, but I doubt it would.) Regex/patterns to match paths is probably the best move to not place requirements on the paths the frontend app uses
  • Can we generate richer tags for users who are authorized? (Are the Open Graph tags useful for authenticated user's browsers at all?)
  • Can we avoid leaving the OG tags in the page when the page URL / view changes away from the original content the tags were generated from? (I don't want to leave false information within the DOM after a state change.)
    • Idea: mark all page-dependent tags with a data attribute and remove them on page load with javascript
  • Can we avoid an extra API call by checking to see if the access level is non-anonymous before making the request to an authenticated endpoint? Yes, done.
  • We should send the first part of the html response before starting the API requests in order to let the browser start preloading Done.
  • Need to handle the case when the backend is offline or unresponsive Done
  • Need to HTML escape external_host_url or request_uri_path to prevent XSS?
    • Potentially also escaping username, tag, pool names, since non-default regexes could allow html escapes to be included in those names

@robobenklein
Copy link
Contributor Author

A tangentially related idea that I had while writing this, since the lua is embedded within Nginx it's possible to return the correct logical HTTP status codes for resources which might not exist.

e.g. making an initial request to /user/does-not-exist could return a 404 status while still serving the single page app, and would allow external sites to better understand the real status of a linked page.

@sgsunder
Copy link
Collaborator

Can't we just add a route to our existing REST application that will generate these stub HTML files instead? It wouldn't require adding a dependency to Lua and it would be more lightweight.

@robobenklein
Copy link
Contributor Author

@sgsunder absolutely possible, I just didn't take that approach because it involved the backend containing some of the code from the frontend

I can try that approach, just need to address the following points:

  • How should I pass the html SPA template to the backend? (Or should it just take ownership of that file entirely?)
  • How should the backend handle templating the html file? (AFAIK it currently only returns json data)

Then I think I could try that approach and compare. Right now before I even have a plan I think it'll turn into a [faster response and larger frontend container] vs [slim frontend and slightly higher response times].

Another point to research is whether the output from the python API can be sent chunked, so that the browser can start loading the SPA assets before all the page tag information has been fetched by the server. (You can see where I do this in the lua by flushing the output.)

@sgsunder
Copy link
Collaborator

@robobenklein I made PR #441 trying this out, I'm open to any feedback and we can discuss which implementation would be more efficient.

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.

2 participants