Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Continuous saving of history #112

Open
AckslD opened this issue Mar 9, 2021 · 17 comments
Open

Continuous saving of history #112

AckslD opened this issue Mar 9, 2021 · 17 comments
Labels
enhancement New feature or request

Comments

@AckslD
Copy link
Contributor

AckslD commented Mar 9, 2021

If I understand it correctly history is only saved on exit. Would it also be possible to save continuously from time to time? Since sometimes if I just shutdown my computer without explicitly closing scli it seems the history is not saved.

@exquo exquo added the enhancement New feature or request label Mar 9, 2021
@exquo
Copy link
Collaborator

exquo commented Mar 9, 2021

Yes, history is currently saved on exit. Being able to protect it from OS crashes, power failures, etc would be an improvement.

@NK308
Copy link

NK308 commented Sep 23, 2021

I guess, making scli react properly to a SIGTERM would already do a lot. Not about crashed, but about beeing shutdown by the terminating desktop session.

@obnoxiousmods
Copy link

obnoxiousmods commented Sep 24, 2021

Might pull request this in as a feature in next few days, the code has no comments and makes it a wee bit hard for me to get a full grasp on mentally and I want to make something that works clean, might not though.

exquo added a commit to exquo/scli that referenced this issue Sep 25, 2021
Attempt to shutdown gracefully on receiving SIGTERM or SIGHUP from another process. This allows to properly save the conversation history.

Ref. isamert#112
exquo added a commit to exquo/scli that referenced this issue Sep 25, 2021
Prevent loosing past conversation history due to system crashes, etc.

Ref. isamert#112
@exquo
Copy link
Collaborator

exquo commented Sep 25, 2021

@NK308, thanks for the suggestion! I've added a handler for the SIGTERM and SIGHUP signals. It attempts to close and save everything gracefully, like it currently does for SIGINT (Ctrl + C).
(The changes are in the develop branch)

Yes, this still does not protect the history file from system crashes or hangups. To limit the damage in those events, I've also added instructions to backup the history file on startup. Now the loss is limited to the last session, rather than the whole JSON file getting corrupted.

A proper long-term solution would be to store the conversations history in a database instead of a JSON file. This would be more efficient and reliable, since the new messages could be added incrementally, rather than the whole JSON structure re-written on every exit.

@exquo
Copy link
Collaborator

exquo commented Sep 25, 2021

@obnoxiousish, the signals handling should be taken care of, AFAICT, but feel free to make a PR with additional changes for this or other issues.

Yes, the code documentation is not extensive. The code itself is somewhat modular - you don't have to understand everything to make improvements - but having an overview of how the pieces fit together helps. Feel free to open a new discussion if you have questions.

@NK308
Copy link

NK308 commented Sep 25, 2021

It seems to work now with the development branch.

Messages now survive, when I log out of i3, or reboot, while scli is still open.

@NK308
Copy link

NK308 commented Oct 3, 2021

I fear, I have to correct my previous statement. Writing messages to myself, between scli and android, leads to a correctly saved chat (which was my way of testing the commit), but chats with other contacts don't seem to be saved, like before.

@exquo
Copy link
Collaborator

exquo commented Oct 7, 2021

For the purposes of saving the conversations history, in scli there should be no difference between messages-to-self and messages to others.

I have just tested sending a message to another number, and issuing a kill -TERM <SCLI_PID> from another terminal. On restarting scli, the message is present in history. So it seems to work fine..

@NK308
Copy link

NK308 commented Oct 7, 2021

I checked again and the problem doesn't seem to be, with whom I'm messaging, but if I just log out of my i3 desktop session, or shutdown/reboot my machine. The exit command from i3, which logs me out of the desktop session and leads me back to sddm seems let close scli properly, while systemctl poweroff/shutdown don't. I actually expected systemctl to give the programs time for this, until it kills them forcefully.

@exquo
Copy link
Collaborator

exquo commented Oct 7, 2021

Normally, on shutdown the system sends a SIGTERM to all processes, and then SIGKILL to those that are still running after a timeout has passed (typical timeout = 90s). There had been a bug in systemd that used to send SIGKILL on shutdown right away, but it should be fixed now.

You can check which signals a process gets on system shutdown with strace. I have just tried the following in a Debian VM:

$ sleep 1000 &
[1] 1234
$ sudo strace -p 1234 -o ~/temp/strace.out &
$ sudo shutdown now
# ... After next startup
$ cat ~/temp/strace.out 
restart_syscall(<... resuming interrupted read ...>) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
--- SIGHUP {si_signo=SIGHUP, si_code=SI_USER, si_pid=2345, si_uid=1000} ---
+++ killed by SIGHUP +++

Apparently, a SIGHUP has been sent instead of SIGTERM. Not sure why..

Note: the above is not about scli specifically. Scli should now handle SIGTERM and SIGHUP properly (i.e. save history). The above steps just check whether a particular OS sending those signals on shutdown.

@NK308
Copy link

NK308 commented Oct 21, 2021

I did tests on various ways to exit the desktop now, under i3 as well as KDE (both under Ubuntu 20.04).
The behaviour on both machines was the same:
systemctrl poweroff, shutdown, shutdown -hP, systemctrl reboot and reboot -hP lead to

restart_syscall(<... resuming interrupted read ...> <detached ...>

so strace itself seems to be cut off, before logging, what happens to the process.

Only exceptions are reboot (without -hP), which leads to

restart_syscall(<... resuming interrupted read ...>) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)

which is still cut off before logging, which signal actually is received.
And logging out of the desktop session and going back to sddm over the tool provided by the window manager (exit-command of i3 and logging out of KDE over the menu) actually leads to a complete log:

restart_syscall(<... resuming interrupted read ...>) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
--- SIGHUP {si_signo=SIGHUP, si_code=SI_USER, si_pid=5884, si_uid=1000} ---
+++ killed by SIGHUP +++

@exquo
Copy link
Collaborator

exquo commented Oct 22, 2021

I've tried some of those shutdown commands on a DebianVM - all gave me the same SIGHUP message as sudo shutdown now above (apparently, systemd sends SIGHUP in addition to SIGTERM on normal shutdown).

Not sure why a processes would be interrupted more abruptly.. Maybe it's the terminal emulator that kills all its child processes? You can try closing all the terminal windows before executing the shutdown command, and executing the shutdown command from either a new terminal or through a GUI menu. Or use nohup / disown. Another option: try it from a TTY (not a GUI terminal emulator); can even stop the DE/WM/X11 processes beforehand.

c-pec pushed a commit to c-pec/scli that referenced this issue Apr 20, 2022
Attempt to shutdown gracefully on receiving SIGTERM or SIGHUP from another process. This allows to properly save the conversation history.

Ref. isamert#112
c-pec pushed a commit to c-pec/scli that referenced this issue Apr 20, 2022
Prevent loosing past conversation history due to system crashes, etc.

Ref. isamert#112
@mark2185
Copy link

Are there any plans to make a periodic history save?

E.g. once a day or something, just so one doesn't have to manually quit and start the chat again?

@exquo
Copy link
Collaborator

exquo commented Aug 28, 2022

This might be a good stop-gap measure until we implement a proper database.

@mlncn
Copy link

mlncn commented Sep 4, 2022

Typing :quit is supposed to be one of the ways that shuts down safely with history saved, yes? Does something have to be configured to enable history?

I have it running via a git checkout, branch master, last commit is 2022-06-03 6ad260c so history save on quit should be working, correct? It is not for me!

@moppman
Copy link

moppman commented Sep 5, 2022

Does something have to be configured to enable history?

See https://github.com/isamert/scli/blob/master/README.md#history

You need to put save-history = true into your sclirc file.

@ghost
Copy link

ghost commented Mar 21, 2023

bumping this issue, as it happened to me multiple times the last days

i'm using a battery powered device, which can run out of power and even if scli was just idling (aka no messages) for a long time, the session history is gone

it would be nice to have the history saved, either on every message (ideal) or every X minutes
i guess a config option could make sense (eg: update-history = [message | minutes])

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants