Skip to content

Implement HTTP caching #3231

Closed
Closed

Description

@jeswr

This would solve...

Performance and robustness issues associated with fetching the same HTTP resource multiple times.

The implementation should look like...

Use an interceptor(?) to implement http caching using the Cache-Control header. I'm inclined to think that the Expires header should not be implemented as http 1.0 has extremely limited use.

Additional context

Originally raised in #3221.

Activity

  1. jeswr commented on May 9, 2024

    @jeswr
    ContributorAuthor

    I've looked into implementing this as an interceptor, and need some information in order to proceed:

    • Is there existing code for caching and cloning responses in the code base that can be used (for this I was expecting to be able to use logic from the web cache which seems to be implementing the Service Worker Spec)
    • Is rfc7234 implemented anywhere in the codebase (I cannot find any indication that it is). http-cache-semantics seems to be a good reference implementation of it if not, however it is under the BSD-2-Clause so @kornelski would need to give permission in order for code to be re-used in this library without copying the BSD licence over.
  2. metcoder95 commented on May 9, 2024

    @metcoder95
    Member

    Is there existing code for caching and cloning responses in the code base that can be used (for this I was expecting to be able to use logic from the web cache which seems to be implementing the Service Worker Spec)

    Can you elaborate on what you have in mind?

    Is rfc7234 implemented anywhere in the codebase (I cannot find any indication that it is). http-cache-semantics seems to be a good reference implementation of it if not, however, it is under the BSD-2-Clause so @kornelski would need to permit for code to be re-used in this library without copying the BSD licence over.

    The closest implementation will be the fetch one but I don't believe it implements its full RFC-7234 spec, so this will be a new implementation for the interceptor.

  3. Uzlopak commented on May 9, 2024

    @Uzlopak
    Contributor

    I have somehow interest to implement it...

  4. jeswr commented on May 9, 2024

    @jeswr
    ContributorAuthor

    Can you elaborate on what you have in mind?

    I was trying to work out the easiest way of caching the response object in-memory would be given that I'm assuming it would be coming through with the body as a readable-stream that needs to be cloned - I was assuming that the Web Cache would already need to implement such behavior. I'll step out of discussions of implementation details now if someone else is willing to take over.

    I have somehow interest to implement it...

    @Uzlopak I'm very time poor so I'm more than happy to step back on my side! Note you'll probably want to implement rfc9111 which appears to obsolete rfc7234.

  5. Uzlopak commented on May 9, 2024

    @Uzlopak
    Contributor

    I am also not very time "rich", but I already investigated some hours in reading into rfc 9111.

    The only thing which was not clear to me, is how we ensure that we dont open a security issue :D

    What I mean is:

    In a browser context you have one user and fetch calls can be cached, as the cache is user scoped. In node we dont have users. So a http cache would be globally activated (?) in nodejs.

    So we need to have something like setGlobalDispatcher but for caching? setGlobalHttpCache?

  6. mcollina commented on May 10, 2024

    @mcollina
    SponsorMember

    I would start having an interceptor or Agent that implement the caching protocol of HTTP, and then worry about how we enable it all the time.

    I think it's possible to have it always enabled and not cache any private data: that's how cdn works after all. Before worrying about this, we need the implementation ;).

  7. Uzlopak commented on May 10, 2024

    @Uzlopak
    Contributor

    Ok, i will start tonight. ;)

  8. kornelski commented on May 10, 2024

    @kornelski

    I've implemented http-cache-semantics by reading the RFC start to finish, and translating each paragraph into code.

    I highly recommend that approach, because it results in good comments with references to their RFC sections (I wish I kept more of them in my code), and helps ensure that unobvious behaviors are not missed.

  9. Uzlopak commented on May 10, 2024

    @Uzlopak
    Contributor

    Usually when I implement an RFC or any other spec I print it out. Then like you say, use the text for commenting the code. When I implemented a paragraph I mark it in the print out that it was implemented, too. Yellow marker for implemented, pink marker for has an issue and should be checked again. When the whole text is colored, then I know the spec is implemented completely ;).

  10. added
    interceptorsPull requests or issues related to Dispatcher Interceptors
    on May 12, 2024
  11. jeswr commented on Jun 26, 2024

    @jeswr
    ContributorAuthor

    Any updates on this?

  12. metcoder95 commented on Jun 30, 2024

    @metcoder95
    Member
  13. metcoder95 commented on Aug 16, 2024

    @metcoder95
    Member

    ping

  14. flakey5 commented on Aug 18, 2024

    @flakey5
    Member

    Hey all, I can start taking a look at implementing this this week. Need to get up to speed on rfc9111 first, but ideally would be able to get a conversation started on some implementation details by end of next week

    Also, related: #2760 #2256 #1146

    cc @mcollina

  15. 16 remaining items

  16. flakey5 commented on Aug 27, 2024

    @flakey5
    Member

    I still didn't get an answer why we need to have a cache key as a string. I think we should keep it maximally flexible and pass in the request options.

    +1

    That won't work though? You can have different request options that have the same result. That's why the spec says you should set the key to method + path and then use the vary header to determine what in the request headers is relevant.

    It allows for each cache store to have its own makeKey (or whatever works best for it). The store gets the full context for a request, can make its own cache key based off of it using what it cares about, and then use it for lookups/puts

  17. metcoder95 commented on Aug 27, 2024

    @metcoder95
    Member

    Response (corrected) Vary can and should invalidate cache if this does not match a already cached requests, and a revalidation should be triggered if the criteria changes.

    This applies if we want to follow the spec explicitly

    +1 on a custom way to generate cache key, only if advising that this will cause the cache to deviate from the spec.

  18. kornelski commented on Aug 27, 2024

    @kornelski

    A request not matching existing Vary doesn't invalidate anything. A response with mismatched Vary invalidates the cache.

    A good HTTP cache should have two cache levels:

    1. Lookup by the URL (plus method if you choose to cache non-GET) to find if the URL is varying

    2. If it's varying, then lookup responses cached for this URL by a key derived from the request header values listed in the cached Vary response header.

    If the second lookup misses, it doesn't invalidate anything yet, only goes to fetch a response. If the server responds with the same Vary, then just add the response to the second-level cache among others (many responses per URL exist).

    Only when the server sends a different Vary header (including lack of one), then discard all existing entries for this URL, and start over with this response.

  19. flakey5 commented on Sep 4, 2024

    @flakey5
    Member

    Just as an update: planning to have at least a draft pr open by this Friday. Not all of the caching directives or optional behaviors will be supported in it just to limit its scope (required ones will be implemented though ofc). Follow up prs will implement more

  20. loynoir commented on Sep 5, 2024

    @loynoir

    Would be nice to see node cache similar to web api cache.

    https://developer.mozilla.org/en-US/docs/Web/API/Cache/matchAll

  21. metcoder95 commented on Sep 5, 2024

    @metcoder95
    Member

    I’d recommend to prioritize first the interceptor’s work, and we can further evaluate how to setup/port Cache as Web API

  22. added 6 commits that reference this issue on Sep 7, 2024
    4bf7c84
    d455642
    4f0ee5b
    9d1cb32
    938fa7a
    f69a8b2
  23. added a commit that references this issue on Oct 14, 2024
    2d40ade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestinterceptorsPull requests or issues related to Dispatcher Interceptors

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions