Login on firefox fails

[chandrin94]

Describe the bug
Hello,

I have a ktor based login page, with username and password, using POST method. It has been working fine for all kweb versions, until 0.8.9. With 0.9.0 and 0.9.1, it still works on chrome based browsers but fails on mozilla firefox with the following stacktrace:

java.lang.IllegalStateException: Unable to find server state corresponding to client id RHnNZs
	at kweb.Kweb.listenForWebsocketConnection(Kweb.kt:287)
	at kweb.Kweb.access$listenForWebsocketConnection(Kweb.kt:44)
	at kweb.Kweb$listenForWebsocketConnection$1.invokeSuspend(Kweb.kt)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
	at java.base/java.lang.Thread.run(Thread.java:829)

Expected behavior
The login should succeed, just like it does in chrome based browsers.

Edit: After checking past issues, this could potentially be related to #191. However, i don't have multiple instances to route traffic into, and session affinity is used anyway.

[chandrin94]

After some search, i noticed that for some reason the clientStateCache (Kweb line 137), does not contain the Client2ServerMessage id, so val remoteClientState = clientState.getIfPresent(hello.id) (Kweb line 286) throws an error.

I am not sure why this happens in firefox specifically, but apparently sometimes the RemoteClientState is not cached correctly.

Modifying

val remoteClientState = clientState.getIfPresent(hello.id)
               ?: error("Unable to find server state corresponding to client id ${hello.id}")

to

val remoteClientState = clientState.getIfPresent(hello.id)
           ?: clientState.get(hello.id) {
               RemoteClientState(id = hello.id, clientConnection = Caching())
           }

seems to be solving the problem, because it caches the RemoteClientState on the spot, if it is not already contained in the cache. However i don't know if this solution is safe, so i would like to hear your opinion.

[sanity]

Hmm, that's strange that it works in Chrome but not FF, can you find any previous reference to the missing client id in the logs prior to the exception?

Client states are dropped by the server after a timeout, but that should be 48 hours so I doubt it's the problem here.

The client state should be created on initial page render here, so that it's present when the websocket connects.

Is it possible the websocket is being broken? I think this change is relatively recent, it will drop the client state when the websocket is broken - whereas previously it relied on the timeout for this. It might make sense to go back to that approach.

Do you see a WS session disconnected for client id: XXX in the logs anywhere prior to the exception?

[chandrin94]

Is it possible the websocket is being broken? I think this change is relatively recent, it will drop the client state when the websocket is broken - whereas previously it relied on the timeout for this. It might make sense to go back to that approach.

@sanity indeed, as a matter of fact i tried reverting the recent change you mentioned back to dropping the client state after a timeout, and yes, it works with the old implementation. The problem, as you mentioned, seems to occur with this recent change.

Do you see a WS session disconnected for client id: XXX in the logs anywhere prior to the exception?

No, but i see this [ERROR] [ktor.application] - Websocket handler failed

[sanity]

Hmm, ok - I'm going to reduce the default timeout to 4 hours and remove that invalidation and release as 0.9.2 in a few minutes.

The original purpose of that was to prevent RAM from filling up with these ClientStates, but I think a better solution might be to make the beforeunload window event send a message to the server explicitly telling it to drop the ClientState.

[sanity]

Ok, just released 0.9.3, would you mind confirming that this solves the problem?

[chandrin94]

Ok, just released 0.9.3, would you mind confirming that this solves the problem?

@sanity Unfortunately the problem still persists for firefox browser. It only works if i completely remove the clientState.invalidate(message.id) in Kweb.kt line 321 and add remoteClientState.clientConnection = Caching() in the finally clause of DefaultWebSocketSession.listenForWebsocketConnection() function (like it was untli version 0.8.9). However i am not sure if what i i did is a proper solution or it just hides the underlying problem.

[sanity]

Hmm, crap - ok, I think I understand the issue, will fix once I get back to my desktop and deploy a new version. Thank you.

[sanity]

Just released 0.9.4, would you mind verifying that this resolves the issue?

[chandrin94]

Just released 0.9.4, would you mind verifying that this resolves the issue?

@sanity I know i'm being annoying at this point, sorry for that, but the problem still persists.
I see you added the remoteClientState.clientConnection = Caching() line i mentioned, but as long as the clientState.invalidate(message.id) line in Kweb.kt line 321 exists the firefox bug persists :(

[sanity]

You're not being annoying at all, I appreciate the assistance with debugging, sorry it's taking so long.

That's confusing though, line 321 should only be executed if the beforeunload event is triggered in the browser, which shouldn't happen until the user is leaving the page.

I'll disable it and release 0.9.5, and create a new issue to investigate.

[sanity]

0.9.5 released, would appreciate it if you could confirm that it works.

[chandrin94]

0.9.5 released, would appreciate it if you could confirm that it works.

Yes, thanks, now i can login from firefox :D .