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

Use a singleton to store the memory cache in an internal WeakMap for the current request #2356

Merged
merged 4 commits into from
Jun 24, 2024

Conversation

jpreynat
Copy link
Member

@jpreynat jpreynat commented Jun 24, 2024

I made the switch from using the ctx property instead of the cf one from the getRequestContext in the getGlobalContext helper while making the memory cache from the middleware accessible to the next request handler.

However, since this change was deployed, we have an increased number of these errors:
Cannot perform I/O on behalf of a different request. I/O objects (such as streams, request/response bodies, and others) created in the context of one request handler cannot be accessed from a different request's handler.

This PR reverts this change, but also switches to use the singleton helper to create the memory cache, that will store the memory cache and makes it accessible in a WeakMap for the current request context.

Copy link

argos-ci bot commented Jun 24, 2024

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 8 changed Jun 24, 2024, 9:06 AM

@jpreynat jpreynat changed the title Replace using ctx by cf property from optional request context Use a singleton to store the memory cache in an internal WeakMap for the current request Jun 24, 2024
@jpreynat jpreynat merged commit f5df40e into main Jun 24, 2024
7 of 8 checks passed
@jpreynat jpreynat deleted the use-cf-instead-of-ctx branch June 24, 2024 09:13
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