When we login to the site in a state that kicks out another state, we get the notice below. It doesn't seem to cause any issues, but I thought I'd report it.

Notice: Undefined property: stdClass::$id in Drupal\session_limit\Services\SessionLimit->_onSessionCollision__DropOldest() (line 278 of /srv/bindings/ed4c2d1db184474bbda099efbd86aeb3/code/modules/contrib/session_limit/src/Services/SessionLimit.php).
CommentFileSizeAuthor
#2 session_limit-notice_error-2787013-02.patch830 bytesnicrodgers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

camprandall created an issue. See original summary.

nicrodgers’s picture

nicrodgers
Primary language English
Location Monmouthshire, UK
CreditAttribution: nicrodgers at Welsh Government commented
Version: 8.x-1.0-beta1 » 8.x-1.x-dev
Status: Active » Needs review
FileSize
session_limit-notice_error-2787013-02.patch830 bytes

Replicated and patch attached.

shrop’s picture

shrop CreditAttribution: shrop at Mediacurrent commented

I think I reproduced this error once, but haven't been able to since. Can you share the settings you are using for Session Limit?

shrop’s picture

shrop CreditAttribution: shrop at Mediacurrent commented

Description

I was able to reproduce the issue after all, but needed to apply the patch from issue 2785937 first.

The submitted patch does resolve this issue.

Testing Instructions

  1. Checkout session_limit 8.x-1.x
  2. Apply this patch
  3. Enable session_limit
  4. Configure session_limit with "Default maximum number of active sessions" set to 1 and "Automatically drop the oldest sessions."
  5. Open two separate browser sessions
  6. Login to browser session 1 with a non-admin test user
  7. Login to browser session 2 with the same non-admin test user
  8. In browser session 1, navigate to another route and you will see the reported notice in Drupal Watchdog

Next Steps

  • Additional review as needed
  • Commit patch to 8.x-1.x
shrop’s picture

shrop CreditAttribution: shrop at Mediacurrent commented
Status: Needs review » Reviewed & tested by the community
nicrodgers’s picture

nicrodgers
Primary language English
Location Monmouthshire, UK
CreditAttribution: nicrodgers at Welsh Government commented

thanks Shrop. I hadn't applied the patch you mention, and it was happening every time somebody logged in which trigged a kicking-out of another session - using the default settings.

shrop’s picture

shrop CreditAttribution: shrop at Mediacurrent commented

Good to know! Well, the patch fixed it for me regardless. Thanks much!

darksnow’s picture

darksnow CreditAttribution: darksnow at Deeson for Deeson commented
Status: Reviewed & tested by the community » Closed (outdated)

The patch above gets the correct session ID from the session, but a more recent version of this code uses $event->getSessionId() so the patch above is no longer needed.

I'm not seeing this error and the code has changed in the mean time so I'm closing this as out of date unless anyone has any objections.