Elixir.FunctionClauseError on Coherence.update_user_logins function

[dskim]

Hi,

I'm just wondering if this is something that needs to be fixed or I'm using it wrong.

I'm not exactly sure how this is happening as I don't see the sender of this request, but I'm getting this exception.

Elixir.FunctionClauseError: no function clause matching in anonymous fn/1 in Coherence.CredentialStore.Server.handle_cast/2
  File "lib/coherence/plugs/authorization/credential_store/server.ex", line 88, in anonymous fn/1 in Coherence.CredentialStore.Server.handle_cast/2
  File "lib/kernel.ex", line 2136, in anonymous fn/2 in Kernel.update_in/3
  File "lib/map.ex", line 791, in Map.get_and_update/3
  File "lib/kernel.ex", line 2136, in Kernel.update_in/3
  File "lib/coherence/plugs/authorization/credential_store/server.ex", line 88, in Coherence.CredentialStore.Server.handle_cast/2
  File "gen_server.erl", line 637, in :gen_server.try_dispatch/4
  File "gen_server.erl", line 711, in :gen_server.handle_msg/6
  File "proc_lib.erl", line 249, in :proc_lib.init_p_do_apply/3
  Module "Elixir.Coherence.CredentialStore.Server", in Coherence.CredentialStore.Server.init/1

We're calling Coherence.update_user_logins(user) when admin updates user's record in order to make sure the logged in users credentials gets updated as well. I think exception is happening due to the fact that the user is not logged in at the time of this update as this code below doesn't seem to handle signed out users.

https://github.com/smpallen99/coherence/blob/master/lib/coherence/plugs/authorization/credential_store/server.ex#L94

I'm wondering if we need to add extra match something like fn nil -> nil to make sure we gracefully handle cases where the user data doesn't present on the GenServer.

Please let me know if this seems like a way to go, I'll submit a PR for that.

Thanks, David

[smpallen99]

I agree. First, there is no reason to call update_user for a logged out user since the purpose of this call is to update the in-memory cache of the user_data. If the user is not logged in, there is no user_data cache for that user.

On one hand, I like the idea of graceful fail, but on the other hand, the exception is showing you that you are doing something wrong (can't update logged out user). So, in a sense, the exception is catching a problem in your application. If we were to fail gracefully, you may never know that the function failed.

Perhaps the way to solve this is to create a second update_user! that throws the exception, with the update_user returning an error tuple if the user is not logged out.

[adz]

Is there a way to tell if a user is logged in?

[dskim]

Hi @smpallen99, Thanks a lot for your reply.

I see what you are saying that we should not try updating logged out users and also the fact that it sort of tells me it is not expected to be used that way.

However, I don't think we're loosing much by making it to fail gracefully when Coherence is not even telling you why it is happening clearly. I think it raises an exception more by accident than by design. It was tricky to find out why as it was happening occasionally when the user being updated was not logged in.

I know it is for updating cache for logged in users, but we're not updating current user's data so more graceful response would be desirable instead of having to check the existence of logged in user's session before.

I'm not sure if I'd ever want it to throw an exception for this even if the exception message was clearer. Probably returning an error tuple would be sufficient in case someone wants to handle it differently.

Also, I'm not sure if there is any method for checking other user's login status though. I guess we can query the database but I'd much rather have it ignore logged out user than querying the db to figure it out. There is also a chance of this exception happening within that split second if the user logs out so I don't think that's the exception free solution either.