#53 Report original exception for session unpickle error

WebKit (45)
Oleg Noga

The purpose of this patch is to provide the original
session unpickle exception instead of KeyError
Since we have no corrupted sessions on the disk, the
Session KeyError is still appears.
KeyError appears if SessionFileStore can't unpickle
session while loading it from disk.
Session can't be unpickled only if it has objects, with
have buggy implemented unpickling feature.
Developer, who works under webkit, would like to fix
unpickling for his session.
So, match better for developer to see the original
exception instead of KeyError.
This patch was made on Webware0.8b1.


  • Oleg Noga
    Oleg Noga

    The patch

  • Oleg Noga
    Oleg Noga

    • assigned_to: nobody --> stuartd
  • Geoff Talvola
    Geoff Talvola

    Logged In: YES

    Wouldn't this part of the patch be easier if we just said
    "raise"? raise with no arguments re-raises the latest
    exception. There's no need to rebuild it from sys.exc_info().

    + e = sys.exc_info()
    + raise e[0], e[1], e[2] # raise the exception again, so
    developers can look on it

  • Geoff Talvola
    Geoff Talvola

    Logged In: YES

    I'm not sure it's a good idea to re-raise the unpickling
    exception here. It may be better to _print_ the unpickling
    exception and possibly email it to the administrator, but
    still raise KeyError so that the application can treat this
    as a missing session and possibly recover more gracefully.

  • Oleg Noga
    Oleg Noga

    Logged In: YES

    Here is KeyError traceback:

    Traceback (most recent call last):
    File ".\WebKit\Application.py", line 368, in dispatchRequest
    elif self.isSessionIdProblematic(request):
    File ".\WebKit\Application.py", line 466, in
    if (time()-request.session().lastAccessTime()) >=
    File ".\WebKit\HTTPRequest.py", line 225, in session
    return self._transaction.session()
    File ".\WebKit\Transaction.py", line 67, in session
    self._session =
    File ".\WebKit\Application.py", line 889, in
    session = self.session(sessId)
    File ".\WebKit\Application.py", line 725, in session
    return self._sessions[sessionId]
    File ".\WebKit\SessionDynamicStore.py", line 72, in
    File ".\WebKit\SessionDynamicStore.py", line 135, in
    self._memoryStore[key] = self._fileStore[key]
    File ".\WebKit\SessionFileStore.py", line 55, in __getitem__
    raise KeyError, key
    KeyError: 20030204232946-

    Seems that KeyError have thrown out of awake-respond-sleep
    cycle (before it), so servlet can’t perform any gracefull
    recoveries. Maybe application must recover the same way as
    when session can’t be pickled? To remove unpickable
    session file, to create new session, and not to throw any
    exception at all?
    Any way, it usefull to see original exception report – in
    console/log, or in http exception report.
    I have raised exception like “raise e[0], e[1], e[2]” just to
    preserve original traceback. If original traceback has no
    helpful information, we can simple “raise <original exception
    The simplest patch is to remove try/except at all :)

  • Logged In: YES

    The original traceback is preserved if you just do "raise"
    without any arguments. It just re-raises the existing
    exception along with existing traceback information.

    From a consistency point of view, raising a KeyError for
    problems retrieving a session seems to be pretty common.
    While I think this specific case is not caught, there are
    other cases where a session stores __getitem__ throws a
    KeyError that does get caught.

    From a users point of view, shouldn't this just appear as a
    missing session file?

    From a developers view, you would want to see what the
    unpickling error was.

    If these two assumptions are correct, then perhaps a
    solution like this:

    - catch KeyError in Application.createSessionForTransaction,
    and treat it like there was no session.

    - have the decoding exception raise a key error as it does
    now, but also print out a the stack trace for a developer,
    possibly calling handleException() to log the exceptino as well.

  • Oleg Noga
    Oleg Noga

    Logged In: YES

    So I've produced patches:
    for Application: Application.diff
    for SessionfileStore.diff

    in SessionFileStore.__getitem__:
    * close session file finally (seems like bug fix)
    in case of session unpicling error:
    1. remove session file
    2. call Application.handleException()
    3. raise KeyError

    in Application.createSessionForTransaction:
    1. catch KeyError
    2. create new session if it was not loaded from store

  • Oleg Noga
    Oleg Noga

    in replace of SessionFileStore2.diff

  • Oleg Noga
    Oleg Noga

    in addition to SessionFileStore.diff

  • new patch works within isSessionIdProblematic()

  • Logged In: YES

    Check out the revised Application2.diff I am uploading.

    This places the catch of the KeyError in
    isSessionIdProblematic() which is called specifically to see
    if a session ID is valid when processing dispatchRequest().
    If it fails (typically because the sesion has expired) it
    marks the session as expired so the servlet can check and
    handle the situation.

    It has the advantage of cleaning up _SID_ cookies and fields
    on the request and going through handleInvalidSession which
    honors the setting 'IgnoreInvalidSession'

    Please advise if this solves your problem. Note it should
    be used in addition to the SessionFileStore.diff patch which
    raises the KeyError.

  • Oleg Noga
    Oleg Noga

    Logged In: YES

    Aplication2.diff in couple with SessionFileStore.diff seems
    works fine.
    Request reports like session expired if session had unpickle
    problems. And we haveoriginal traceback in console.

    • status: open --> closed
  • Logged In: YES

    Committed fix to CVS.