fix: limit proxy session warning to once per client instance

[j4w8n]

What kind of change does this PR introduce?

Bug fix.

What is the current behavior?

A call to getSession(), when using server storage, logs a warning every time session.user is accessed. This is causing a lot of people to see multiple consecutive logs to the console - especially for SvelteKit users.

What is the new behavior?

Ensures that accessing session.user, from a getSession() call, only logs the warning once per Proxy session instance. In other words, once per server request.

Additional context

#888

[Zanzofily]

I believe you are just updating the local variable here, which won't be picked up on next invocations.

[j4w8n]

I believe you are just updating the local variable here, which won't be picked up on next invocations.

Perhaps I got it wrong, but it passes the addition to the test. That let creates a reference to this.suppress... on the client; so it should be changing the value of this.supress... when suppressWarning is changed. Then, when the value is checked again, only for the current server client, it should be true and not emit any further warnings - again, only for the current client.

This is only a per-request "fix".

[Zanzofily]

I believe you are just updating the local variable here, which won't be picked up on next invocations.

Perhaps I got it wrong, but it passes the addition to the test. That let creates a reference to this.suppress... on the client; so it should be changing the value of this.supress... when suppressWarning is changed. Then, when the value is checked again, only for the current server client, it should be true and not emit any further warnings - again, only for the current client.

This is only a per-request "fix".

References only work for objects in javascript, It's passing the test because you're using the value of session?.user rather than re-calling getSession() inside your test

[j4w8n]

I believe you are just updating the local variable here, which won't be picked up on next invocations.

Perhaps I got it wrong, but it passes the addition to the test. That let creates a reference to this.suppress... on the client; so it should be changing the value of this.supress... when suppressWarning is changed. Then, when the value is checked again, only for the current server client, it should be true and not emit any further warnings - again, only for the current client.

This is only a per-request "fix".

References only work for objects in javascript, It's passing the test because you're using the value of session?.user rather than re-calling getSession() inside your test

Ah thank you! I'll have to play with it a bit more then.

So, if I understand you, this is only a partial fix: it works only when you re-access session.user, but if you were to call getSession again, then it would log again.

[kangmingtay]

So, if I understand you, this is only a partial fix: it works only when you re-access session.user, but if you were to call getSession again, then it would log again.

yup that's because a new proxy is created everytime getSession is called and returns the non-expired session

[j4w8n]

Thanks @kangmingtay and @Zanzofily. I've made a couple of tweaks that I believe resolves the issue with subsequent proxy instance creations.

@kangmingtay, I'm not sure if the team approves of me expanding an existing test. I can separate if needed.

[kangmingtay]

@j4w8n that's fine, thanks for helping with this!