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

Data value vanish after first cached value read access #163

Closed
Inoir opened this issue Mar 5, 2022 · 8 comments · Fixed by #164
Closed

Data value vanish after first cached value read access #163

Inoir opened this issue Mar 5, 2022 · 8 comments · Fixed by #164

Comments

@Inoir
Copy link

Inoir commented Mar 5, 2022

Setup:

const instance = setupCache(axios.create(), { ttl: 2000 });

so using the internal memory storage. didnt test any other storages.
so first excution of the request is fine.
2nd is also good, after this the data value vanishes somehow out of the storage without trigger any function call on the storage itself (did console logs on find, set, remove => not getting called)

So this may be project specific, casue we are working alot with references in our system. Since references are totaly fine, there should be an mechanism to store and get an shallow copy of the data value.

So for now my current workaround for this is make an own memory storage:

const CACHE = new Map();
const STORAGE = buildStorage({
  find: (key) => {
    const found = CACHE.get(key);
    if (found?.data) {
      return { ...found, data: JSON.parse(JSON.stringify(found.data)) };
    }
    return found;
  },
  set: (key, value) => {    
    if (value?.data) {
      CACHE.set(key, {
        ...value,
        data: JSON.parse(JSON.stringify(value.data)),
      });
    } else {
      CACHE.set(key, value);
    }
  },
  remove: (key) => {
    CACHE.delete(key);
  },
});

const INSTANCE = setupCache(axios.create(), {
  ttl: 2000,
  storage: STORAGE,
});
@Inoir
Copy link
Author

Inoir commented Mar 5, 2022

Okay found out the problem is, if you send ALOT requests simultaneously, the start of an request and the end overwrites the data result. so in my example, we send a user load unneeded often. was kinda 100x in under 1 sec. and same the the response comes in from the first request, there is still requests going out, so this results somehow in overwriting the data value. receives correct data on first response, all other response have an empty value. think also important to mention, the empty value is an array.. so not empty, but on first response it got data in, 2nd the array is empty, cause reference got modified in this time (?!) only an idea, but not sure.

this feature mentioned above is still important, cause if you modify i.e an array, cause you have other data flows depending on this modification, only a shallow copy should be stored.

EDIT: screenshot here, maybe easier to see whats going on. and maybe also catchable seems the cache is running async to the ongoing response flow?!

Screenshot 2022-03-05 at 18 45 45

@arthurfiorette
Copy link
Owner

arthurfiorette commented Mar 5, 2022

Hey! Thanks for creating this issue. I'm in another city today... If you could create a unit test representing this error it would make it a lot easier! I can work on it tomorrow.

@arthurfiorette
Copy link
Owner

arthurfiorette commented Mar 7, 2022

Hello again! Without code examples, it's hard to reproduce your bug...
I couldn't reproduce it, until I thought of pasting your problem as a comment and letting Github Copilot do its work. And it worked! I have a failed unit test to fix!

This is the test:

it(`tests ${name} to not allow changes by value reference`, async () => {
const storage = Storage();
await storage.set('key', {
state: 'cached',
createdAt: Date.now(),
ttl: 1000 * 60 * 5, // 5 Minutes
data: { ...EMPTY_RESPONSE, data: 'data' }
});
const result = (await storage.get('key')) as CachedStorageValue;
expect(result).not.toBeNull();
expect(result.state).toBe('cached');
expect(result.data.data).toBe('data');
// Deletes the value
delete result.data.data;
// Check if the value has been modified
const result2 = await storage.get('key');
expect(result2).not.toBeNull();
expect(result2.state).toBe('cached');
expect(result2.data?.data).toBe('data');
});

@arthurfiorette
Copy link
Owner

arthurfiorette commented Mar 7, 2022

Instead of just using JSON.parse and JSON.stringify, it may use the modern structuredClone if available.

Can you confirm that this test + fix was your problem?

@arthurfiorette
Copy link
Owner

@Inoir ?

@arthurfiorette
Copy link
Owner

I think that it fixed the current problem. For any future problem, you can reopen this issue or create another one. Bye!

@Inoir
Copy link
Author

Inoir commented Mar 15, 2022

Sorry ill look into this now, haven't had time for it yet.

@Inoir
Copy link
Author

Inoir commented Mar 15, 2022

Yes this is fixed, works nice! thx alot!

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 a pull request may close this issue.

2 participants