deltachat-core-rust
deltachat-core-rust copied to clipboard
fix: Emit WebxdcInstanceDeleted events when deleting a chat (#6670)
Not sure that emitting events optimistically is a good solution, but emitting them after the transaction (as we do everywhere) is also not reliable if the program crashes before that. It seems that the only proper way is to introduce persistent events which are removed by the app from db after processing. Returning an indefinite number of MsgId-s from the transaction may eat up memory, but adding LIMIT to SELECT makes the fix just useless.
Fix #6670
~~> is also not reliable if the program crashes before that.~~
~~do you mean the function "crashing"? if core crashes all events are lost anyway and on a restart it loads the most recent state, so not sure I understand the problem case you are describing.~~
nvm I forgot it is about one of the few events with side effects
To protect against crashes we could alternatively save a list of message ids to delete somewhere in the db and add an api to retrieve them on appstartup and when there are WebxdcInstanceDeleted events
To protect against crashes we could alternatively save a list of message ids to delete somewhere in the db and add an api to retrieve them on appstartup and when there are WebxdcInstanceDeleted events
i do not think, that effort is needed in practise.
when the app crashes the webxdc window etc is usually also removed. might be cornercases left for notifications or icons not being removed, but that alone would not justify keeping events in database (i know, there is some incentive to do that for $reasons, but that is out of scope of this PR)
in general, it should not be the end of the world when an event is missing after program start
~~i think, it is no secret that i am sceptical about any event causing additional effort or database calls anyways :)~~ EDIT: that's offtopic :)
deltachat desktop and deltachat tauri create and store different browser sessions/data directories for each account and on dc tauri even for each webxdc.
So loosing the event will mean there is a folder not cleaned up until you delete the account. (webxdc's data is still on file system)
@r10s I think mobile(iOS and android) just uses different origins for isolation. I don't know if mobile removes the data from a webxdc at all, after the message is deleted, so might not apply to mobile, yet.
well, data is deleted after 30 days or so. not sure about indexdb, which however is not used much in practise. compared to all the overhead, for now, we can ignore that issue
for desktop, it seems be easy enough to iterate over the dirs and delete the ones without a message. but also there, it seems fine to accept some overhead for now.
remember, we're talking about app crashes at exact time of delition, better spend time to fix crashes (have we seen any crash in that area?)
well, data is deleted after 30 days or so
where do you get that number from?
have we seen any crash in that area?
not that I know of, it seems to be theoretical. seems like @iequidoo brought it up as potential issue. I agree that we should ignore the theoretical crash issue for now
So loosing the event will mean there is a folder not cleaned up until you delete the account. (webxdc's data is still on file system)
If so, i think that the app should first list webxdc messages in the chat, then delete all associated data, then messages themselves (all this can be done in several iterations if there are many webxdc messages) and only then delete the chat itself, i.e. the app shouldn't rely on events (which still can be emitted at least for feature-completeness). If mobile apps don't need to clean up anything additionally, they can just delete a chat as currently.
don't forget that message deletion is synced between devices, we still need the event.
don't forget that message deletion is synced between devices, we still need the event.
True, so we anyway need to add some kind of persistent events to make webxdcs deletion reliable at least for Desktop. There are other ways in which messages may be deleted, e.g. delete_device_after may be set. Let's not fix this for now
well, data is deleted after 30 days or so
where do you get that number from?
some ppl were complaining that eg, wonster stats are gone after not opening the app some time :)
afaik, there is no "hard timelimit", though. but on "storage pressure" browser/webview seem to clean up
we discussed that several times, agreeing that for now, it is fine. eg. spending our resources into deduplication probably saves 10-100 times more storage than thinking too much about localstorage/indexdb. the vast majority of cases is small data, if there is sth left, then it's that, at some point it will be cleaned 🤷♂️
and yes, i agree on ignoring the issue for now to not complicate things.