mirror of
https://github.com/BookStackApp/BookStack.git
synced 2026-02-07 03:09:44 +03:00
Lost 23 hours of notes b/c it incorrectly indicated that the draft was saved #1761
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @bobjunga on GitHub (Jun 5, 2020).
I was pressing cntr-s and believed that I was getting positive feedback that it was saving but after a short network problem, I found that the last saved draft was 23 hours old.
I suspect that you need to change the browser side code to require positive confirmation of a save rather than taking the absense of an error as indication of success.
Also, you could save each cntr-s draft as a separate row and instead of deleting them, set a deleted column to true. Then use a cron job to remove deleted row that are over N days old.
Steps To Reproduce
Steps to reproduce the behavior:
Expected behavior
Not to have my hard work pissed away!
Your Configuration (please complete the following information):
Additional context
Discord transcript:
bobgToday at 11:56 AM
I just lost a day of research notes! I was hitting cntrl-s often, and I know that I saw a msg about the page being saved, yet when I just had a network blip, my page went back to empty! I queried page_revisions table and found only the initial save with a fraction of the content. Is there anywhere else I can look to find a more recent version of the content?
the current page is actually blank so I suspect that when I hit 'back' in the browser it resulted in the browser having an empty page and then submitting that empty page
any ideas?
been using bookstack for a week and was loving it up to now 😦
bobgToday at 12:07 PM
what is the concept of a "draft"? Is that stored in a different place than a "revision"? Experimenting on a different page, the message I remeber seeing was "draft saved"
bobgToday at 12:16 PM
weel, I dumped the entire DB and serached for keywords -- the draft is not in the DB anywhere.
I assume thats it. no where else it could be. I would appreciate any confirmation of that before I move on.
bobgToday at 12:34 PM
So now I do see that initial commit of the page with some data in the page_revisions table but when I go to revisions in the UI, even though It lists one revision clicking on the "changes" and on "current" results in no content being shown? I can get the content string directly from the DB, how do I get that back into my page?
ssddanbrownToday at 12:45 PM
@bobg A revision is a published version of the page. A draft is user-specific draft content for a page.
What type is the row in the DB where you've found your content?
bobgToday at 12:46 PM
I think its a update_draft
ssddanbrownToday at 12:47 PM
Okay, There should be a user_id or something on that row, does it match the id of your user?
bobgToday at 12:47 PM
(I am using the mysql cmdline and the formatting of columns in hard to line up)
yes -- created_by = 3 which is me
ssddanbrownToday at 12:52 PM
I'd expect that content to be restored into the editor when you go to edit the page again, Should pop up a message like "Here's content you were editing earlier" .
bobgToday at 12:52 PM
oh. maybe if I just hit "edit" in the UI it will populate with my draft?
ssddanbrownToday at 12:53 PM
Should do, Make sure you have a copy of that DB first, just in case
bobgToday at 12:53 PM
good idea:) -- will do
that 'worked"
this draft was from 23 hours ago it says:(
so why was it letting me press cntr-s for a day without actually saving the draft?
I can not say for sure that I was seeing the "draft saved" message but I am pretty sure that I was -- I know it registered that cntr-s was doing something
after the network blip, I got a message about not saving so after network was back, I hit "save page" and that got a "4xx|Page Expired" msg. (I think it was 417)
I can not remember the last time I had a content lost error like this. It is painful because I was keeping really good notes!
but anyway, thanks for your help
ssddanbrownToday at 1:37 PM
@bobg Yeah, That's a harsh situation , apologies. I think someone else did report something similar but I had trouble re-producing it. Might need to double check the autosave logic to ensure we're always throwing errors when needed, think some cases have appeared where the save fails but a garbled success message pops up.
Autosave can always be tricky there as there's the risk of autosaving bad content off the back of another issue.
If you can re-produce the scenario in any manner make sure to file a bug so we can address it.
bobgToday at 1:44 PM
yes, after I understood the draft vs versions, I assumed that the problem would be that it had been saving but the last save was empty. I am very surprised to find that it had not had a succesfull save in 23 hours
in what ever IPC you are using. the browser side thought it was a success when the server side was not successfull. That should be solved in the protocol so that it can not happen. the browser needs a positive confirmation of success as opposed to a lack of an error.
I actually thought that cntr-s was the same as "page save" + "edit". There was no indication that saving in that manner was more risky.
bobgToday at 2:00 PM
each draft save could produce an update_draft+(countCol++) revision and instead of deleting them on "page save", you just toggle a deleted column on all those rows. Then a cron job removes deleted drafts older than N days.
@bobjunga commented on GitHub (Jun 5, 2020):
I found a partial reproducible issue. This is not exactly what happened to me because mysql did not stop running during the time of my problem, but it illustrates an important flaw in the application that a failure to save draft content is not conveyed to the user. If this issue is fixed, it would prevent the loss of data that I suffered.
What happens is that a green check momentarily appears at the top of the UI, which seems to indicate that the page is saved. The text next to the checkmark is "Undefined NaaN.NaN" instead of the last saved time but its formatted the same as the time stamp would have been and its easy not to notice. What catches your eye is the green check mark appearing and then disappearing.
What should happen is a prominent UI indication that the content was not saved. For example, that "last saved" element should be red and loud with the text "could not save". It would also make sense to inhibit further changes to the content but that's just a nice to have.
That checkmark should only appear in response to positive round trip confirmation of the save.
(BTW, this actually cost me two days because I could not let go of this loss and spent a lot of time looking into it)
--BobG
@bobjunga commented on GitHub (Jun 6, 2020):
I've been editing the page over time and it got into that state again. I was away for a couple hours and the computer was on the whole time. It was working before but when I came back I started entering additional content and soon noticed that it was not saving anymore.
When you see "undefined 0NaN:0NaN", if you press "save page" you will immediately loose any unsaved work. The work-a-round is to...
When I press "save page" on the original tab I get "419|Page Expired"
My guess is the the server expires the page after a fixed period of time that it is open for editing. At that time any further request in that page context fails but the XMLHttpRequest that implements the draft save ignores the error and reads response data that is not there so when it updates the draft element, the null data renders as "undefined 0NaN:0NaN".
The "save page" is apparently implemented differently and correctly displays the page expired error.
If the save draft is fixed to notice the page expire error, it can just display the page expired message similar to the "save page" request. There might be a little unsaved data at that time but the really important thing is to prevent the user from continuing to enter content. Ideally you could freeze editing but still leave the page so that the user could copy and paste the data.
Of course, its possible to get the server to renew the page session and continue to save but the more important fix is to make sure that if the save fails for any reason, the page will be in a state that it no longer accepts new content from the user or at least prominently warns the user that the page is not in a good way.
I have also observed that it seems to be autosaving frequently so that the cntr-s is not really necessary. That means that even if the user looses the unsaved changes, its not that much as long as the UI informs the user as soon as the page is expired.
BTW, I really like having the manual cntr-s save and the positive feed back of the green checkmarkin response to saving the draft -- as long as it really is a positive confirmation that it was saved.
--BobG
@bobjunga commented on GitHub (Jun 8, 2020):
Here is another scenario where you can loose edits in progress.
Work-a-round:
It seems that what ever chrome does to restore the page ends up loading the last revision instead of your draft content. I suspect that the URL that chrome submits is either different than the "edit" action or the state of the authentication/session is not complete when it makes the request.
--BobG
@hacketiwack commented on GitHub (Jun 9, 2020):
Experienced a similar problem here. I suspect the session was expired, but no warning was displayed.
As written by others above, the page was marked as saved and then clicking on the save button gave the 419 Page expired error.
@phylor commented on GitHub (Jun 15, 2020):
Experienced the same and also lost a lot of text in the process 😞 The UI told me that drafts have been saved and the draft timestamp was up to date. A couple of hours later, I clicked on
Save pageand got the419error. After that, all changes were gone.@ssddanbrown commented on GitHub (Sep 19, 2020):
Apologies to everyone that encountered this.
Some changes to how we make AJAX requests caused errors to not be thrown when expected leading to a misleading success message showing instead of a warning on error message. For v0.30 quite a lot of the JS has been touched upon which included updating the AJAX logic to properly throw errors when requested.
I also done further testing myself and added a safety layer in
d9e2bddee4to always show an error on failed requests (Sometimes only the first error in a series would be shown before) in addition to storing the page content in the browser's LocalStorage upon error event to give a chance of possible manual recovery where important.Again, My apologies and sorry this hasn't been addressed sooner, the last few months have been hard going leaving little time to work on the project. These changes will be part of v0.30 which I'm intended to release this weekend.
@phylor commented on GitHub (Sep 21, 2020):
@ssddanbrown Thank you for the update and all the work! 👍