don't delete saved session on crash

[yogo1212]

For debugging, i added a bit more output to qutebrowser.
The restart failed because i had forgotten to change my editor from tabs to spaces.
TabError: inconsistent use of tabs and spaces in indentationTabError: inconsistent use of tabs and spaces in indentation

After fixing that, qutebrowser loaded an empty session.
Oops. 60 tabs gone.
Only the new tab in ~/.local/share/qutebrowser/sessions.

I don't think that should be expected to happen.
If it is, can qutebrowser make a backup of the old session file before overwriting it?

Reproducible.

qutebrowser v1.6.0
Git commit: 
Backend: QtWebEngine (Chromium 69.0.3497.128)

CPython: 3.7.2
Qt: 5.12.1
PyQt: 5.12

[arza-zara]

Do you have auto_save.session set to true? Anyway, I'd like to keep a number of sessions as a backup.

[jgkamat]

Leon George writes:

For debugging, i added a bit more output to qutebrowser. The restart failed because i had forgotten to change my editor from tabs to spaces. `TabError: inconsistent use of tabs and spaces in indentationTabError: inconsistent use of tabs and spaces in indentation` After fixing that, qutebrowser loaded an empty session. Oops. 60 tabs gone. Only the new tab in `~/.local/share/qutebrowser/sessions`. I don't think that should be expected to happen. If it does, can qutebrowser make a backup of the old session file before overwriting it? Reproducible.

I cannot reproduce, the _autosave session gets loaded correctly after a crash, both with/without auto_save.session for me.

[toofar]

Can you say where the IindentationTabError was thrown from? And I am assuming it was the default session.

[yogo1212]

@toofar i edited qutebrowser/browser/webengine/webenginetab.py to debug whether skype was actually requesting permissions or just pre-emptively complaining.

@arza-zara auto_save.session is true. but it doesn't affect the behaviour in any way (just like with @jgkamat ).

i use the default qutebrowser from arch-linux. no other files modified.

this is my whole config:

c.auto_save.interval = 60000
c.auto_save.session = True
c.session.lazy_restore = True

c.content.host_blocking.enabled = True

c.editor.command = ['konsole', '-e', 'vim', '{file}', '+{line0}']
c.editor.encoding = 'utf-8'

config.bind('<Ctrl-O>', 'enter-mode passthrough')
config.unbind('<Ctrl-V>')

[yogo1212]

here's a script to reproduce:

# close qutebrowser instances
qutebrowser :wq

# make sure this is the right directory if you want your session saved
cp ~/.local/share/qutebrowser/sessions/default.yml /tmp/qb-session-$(date +%s).yml
# other files probably work just as well
file=/usr/lib/python3.7/site-packages/qutebrowser/browser/webengine/webenginetab.py

# introduce syntax error
sudo sh -c "echo GARBAGE >> $file"

qutebrowser &
pid=$!
# close qutebrowser when it shows the bug report window
sleep 3 ; kill $pid

# resolve syntax error
sudo sed -i '/GARBAGE/d' $file

qutebrowser

[The-Compiler]

I can't reproduce this even with the script. The saved default.yml gets loaded as expected and I end up with the pages I had when doing :wq.

[arza-zara]

I can reproduce with this in source directory:

qutebrowser="./.venv/bin/python3 -m qutebrowser --debug -B $HOME/qutedir/"
file=./qutebrowser/browser/webengine/webenginetab.py

$qutebrowser -R ./tests/end2end/data/numbers/{1,2,3}.txt &
sleep 2
$qutebrowser :wq

echo GARBAGE >> $file
$qutebrowser

sed -i '/GARBAGE/d' $file

Crash reporter shows Open Pages as empty. Selecting [x] Restore open pages and Don't report opens only url.start_pages. :session-load default reloads the saved session.

[yogo1212]

@The-Compiler would you be interested in debugging remotely?

@arza-zara if you close the dialog using ctrl-c in the terminal instead of using the buttons, does it restore the session?

[arza-zara]

@arza-zara if you close the dialog using ctrl-c in the terminal instead of using the buttons, does it restore the session?

If I close the crash dialog with ctrl+c in terminal (or unchecking Restore open pages and clicking Don't report), qutebrowser closes, correct default.yml session file is preserved and then opening a new instance restores the default session correctly.

[jgkamat]

arza writes:

I can reproduce with this in source directory: ``` qutebrowser="./.venv/bin/python3 -m qutebrowser --debug -B $HOME/qutedir/" file=./qutebrowser/browser/webengine/webenginetab.py $qutebrowser -R ./tests/end2end/data/numbers/{1,2,3}.txt & sleep 2 $qutebrowser :wq echo GARBAGE >> $file $qutebrowser sed -i '/GARBAGE/d' $file ``` Crash reporter shows Open Pages as empty. Selecting `[x] Restore open pages` and `Don't report` opens only `url.start_pages`. `:session-load default` reloads the saved session.

I can reproduce with this method. The session is not loaded, but not overwritten either, and loading the 'default' session will restore the saved page. I think the reason why the crash restore didn't work is because the _autosave session (which is used for crash recovery ?) was not saved as the crash is too early for that to happen. FWIW, I would really like some style of log rotation on the autosave session files, so even if things got overwritten, I would have <x> saves to notice (not much help right now, but would help a lot after #4545 with a high timeout).

[yogo1212]

i'm not sure any of your problems have much to do with mine (my session file isn't preserved and restarting qutebrowser a second time doesn't change anything either).

what i find unintuitive about the behaviour on my computer is:
why does a parsing error burn a session?
why is the session file altered despite me pressing ctrl-c and not choosing recovery?

i see now that the 'recovery' can't be implemented by 'not overwriting' the session.
the design of the bug report window aims at crashing with an active session loaded.
the next thing that comes to my mind is detecting whether there is sane currently loaded session and not performing the recovery when there isn't one.

i'm glad you're all here looking at historical session files and different settings - but i don't see those issues as being related to mine.

when i start qutebrowser with a session in the file system, encounter a crash, and terminate the process, i don't expect the session to be altered in any way.

[yogo1212]

i'm not sure any of your problems have much to do with mine (my session file isn't preserved and restarting qutebrowser a second time doesn't change anything either).

what i find unintuitive about the behaviour on my computer is:
why does a parsing error burn a session?
if i close the crash window with ctrl-c i haven't made any choice about my system and expect nothing to change.

historical session files are separate issue, imho

[yogo1212]

i've added log to find out when exactly the session file is altered:
log.txt

before start with crash
b4242065d2d735cfda60d70c90d58a8b  

after start with crash
b4242065d2d735cfda60d70c90d58a8b  

before restart
b4661f7f684911cd12c261d44377a399  

after restart (still running)
b4661f7f684911cd12c261d44377a399

after closing restarted with :q
1b1a7e11004f49bb55ba17d195212126  

the session file isn't altered until the instance that was started after the crash window is closed again.

[The-Compiler]

From @toofar in IRC:

I think I lost a session recently because I compiled Qt wrong and the xcb qpa plugin didn't get compiled. So removing that and setting the autoload setting and waiting a bit after the error is printed to the log might so it.

[PRESFIL]

Sessions are also deleted if you turn off qutebrowser normally, but immediately
after starting. After that, the session file is like this:

windows:
- active: true
  geometry: !!binary |
    AdnQywADAAAAAAAAAAAAFQAAB38AAAQ3AAAAAAAAABUAAAd/AAAENwAAAAAAAAAAB4AAAAAAAAAA
    FQAAB38AAAQ3
  tabs:
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []
  - history: []

All tabs is blank 😕

[The-Compiler]

@PRESFIL that's a slightly different issue, due to #5359 and #2176.

[PRESFIL]

I don’t know, there are too many similar problems here, it’s very similar in description.
Also looks like a duplicate #3918.

[The-Compiler]

I don't see how #3918 would be related at all.

[toofar]

Adding some more details for an example of this issue that relates to the autosave session and an early crash:

1. the autosave session is saved every time a load_finished signal is emitted
2. saving the autosave session is throttled to only occur every 60s max
3. when you are loading a session the autosave session will be saved when the first tab has loaded, if some of the other tabs are still early enough in their initialization they will be saved to the autosave session with urls of just '', an empty string.
4. since empty URLs are invalid you now have an invalid autosave session on disk. If the browser crashes in that first 60s (eg via :debug-crash), and you choose not to restore tabs from the crash dialog then next time you start the browser it'll try to load that invalid autosave session
5. at that point it should print Failed to load session _autosave: PyQt6.QtCore.QUrl('') is not valid to the error log and leave you with the default new instance state which is probably a window with your start pages in it (duckduckgo by default)
6. while it looks like your session is lost it actually still exists on disk, it was never loaded because the autosave session took precedence. You can load it with :session-load
7. if you don't manually load your session and instead quit the browser it will override your session on disk
   • if you have auto_save.session = true (which I suspect most people running into this do) then a plain :quit will overwrite your session
   • otherwise if you are in the habit of quitting with :quit --save (aka ZZ or :wq) that'll overwrite your session anyhow

There are a number of things I think we can do to mitigate this:

1. don't save sessions with invalid URLs, since we can't load them anyway
   • just doing this naively will mean you always get an error log at startup when loading a session saying the first autosave attempt fails, that seems annoying
   • a mitigation to that might be to only start autosaving after everything is loaded for the first time. But I think tracking that state is actually quite difficult with special pages having different load signals to normal pages and recurring issues with page lifecycles and signals not being emitted with different Qt versions
   • so while this is probably the proper fix, it's not a clear path forward
2. ignore session entries with invalid URLs
   • this doesn't resolve the issue, you'll still just end up with just one tab loaded
3. if there is an autosave session but loading it fails, log an error, back that file up, proceed with loading the default session
   • this might not fix the cause of the issue but I think it is pragmatic and easy
4. prompt the user with details of the situation and ask what to do (Ask before restoring autosave session #5461)
   • likely this would only be done in combination with one of the other options anyhow. Because some people would want to set a default action without a prompt, and they they would probably still be able to run into this issue
5. keep around multiple session files (Keep around multiple session files #5098)
   • this helps with the general case of getting back a session file you overwrote, it doesn't do anything to prevent you from overwriting a file when you didn't intend to in the first place

Here is a commit that does (3). We don't seem to have tests in this area (Edit: looks like this patch on its own creates an empty windows from the aborted load attempt which needs to be cleaned up):

commit c3396ab3ccb8198041a82634392d700c096950b7
Author: toofar <toofar@spalge.com>
Date:   Wed Dec 27 11:48:19 2023 +1300

    Fall back to loading default session if autosave fails
    
    If loading the autosave session fails you can be left with a browser window
    with just your start pages in it, instead of the default session that is
    normally loaded on startup (assuming you have `auto_save.session = true`). It
    is then quite easy to overwrite your default session if you aren't careful.
    
    This is particularly helpful when an early crash caused an invalid autosave
    session to be saved, which we later fail to load. See https://github.com/qutebrowser/qutebrowser/issues/4672#issuecomment-1869807246
    
    If the default session then fails to load the user will be back in the current
    state of having a single browser window with their start pages open.
    
    After this there will be a `_autosave.yml.bak` file saved to disk. Although
    there is nothing to make that obvious to the user.

diff --git a/qutebrowser/misc/sessions.py b/qutebrowser/misc/sessions.py
index c8445e4ad4d4..162398ccf80d 100644
--- a/qutebrowser/misc/sessions.py
+++ b/qutebrowser/misc/sessions.py
@@ -723,12 +723,17 @@ def load_default(name):
     Args:
         name: The name of the session to load, or None to read state file.
     """
-    if name is None and session_manager.exists('_autosave'):
-        name = '_autosave'
-    elif name is None:
-        try:
-            name = configfiles.state['general']['session']
-        except KeyError:
+    try:
+        last_session = configfiles.state['general']['session']
+    except KeyError:
+        last_session = None
+
+    if name is None:
+        if session_manager.exists('_autosave'):
+            name = '_autosave'
+        elif last_session is not None:
+            name = last_session
+        else:
             # No session given as argument and none in the session file ->
             # start without loading a session
             return
@@ -739,6 +744,19 @@ def load_default(name):
         message.error("Session {} not found!".format(name))
     except SessionError as e:
         message.error("Failed to load session {}: {}".format(name, e))
+
+        if name == "_autosave":
+            message.error(
+                "Attempting to save backup of autosave and trying main session"
+            )
+            path = session_manager._get_session_path("_autosave")
+            try:
+                shutil.copyfile(path, path + '.bak')
+            except OSError as err:
+                message.error(f"backing up autosave failed: {err}")
+                pass
+            session_manager.delete_autosave()
+            return load_default(None)
     try:
         del configfiles.state['general']['session']
     except KeyError: