#53 Report original exception for session unpickle error

closed
WebKit (45)
5
2003-02-18
2003-01-31
Oleg Noga
No

The purpose of this patch is to provide the original
session unpickle exception instead of KeyError
exception.
---
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.

Discussion

1 2 > >> (Page 1 of 2)
  • Oleg Noga
    Oleg Noga
    2003-01-31

    The patch

     
  • Oleg Noga
    Oleg Noga
    2003-01-31

    • assigned_to: nobody --> stuartd
     
  • Geoff Talvola
    Geoff Talvola
    2003-02-04

    Logged In: YES
    user_id=88162

    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
    2003-02-04

    Logged In: YES
    user_id=88162

    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
    2003-02-04

    Logged In: YES
    user_id=551440

    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
    isSessionIdProblematic
    if (time()-request.session().lastAccessTime()) >=
    request.session().timeout():
    File ".\WebKit\HTTPRequest.py", line 225, in session
    return self._transaction.session()
    File ".\WebKit\Transaction.py", line 67, in session
    self._session =
    self._application.createSessionForTransaction(self)
    File ".\WebKit\Application.py", line 889, in
    createSessionForTransaction
    session = self.session(sessId)
    File ".\WebKit\Application.py", line 725, in session
    return self._sessions[sessionId]
    File ".\WebKit\SessionDynamicStore.py", line 72, in
    __getitem__
    self.MovetoMemory(key)
    File ".\WebKit\SessionDynamicStore.py", line 135, in
    MovetoMemory
    self._memoryStore[key] = self._fileStore[key]
    File ".\WebKit\SessionFileStore.py", line 55, in __getitem__
    raise KeyError, key
    KeyError: 20030204232946-
    b46a22352890568b1a899c361d816933

    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
    instance>”
    The simplest patch is to remove try/except at all :)

     
  • Logged In: YES
    user_id=326269

    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
    2003-02-05

    Logged In: YES
    user_id=551440

    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
    2003-02-05

    in replace of SessionFileStore2.diff

     
    Attachments
  • Oleg Noga
    Oleg Noga
    2003-02-05

    in addition to SessionFileStore.diff

     
    Attachments
  • new patch works within isSessionIdProblematic()

     
    Attachments
1 2 > >> (Page 1 of 2)