[BUG] Data loss on Node if one or more modified files isn't writable

[hoelzro]

Describe the bug See https://groups.google.com/forum/#!topic/tiddlywiki/Y0fLouDHCqU

If one or more files underneath the wiki's tiddlers directory isn't writable, and one of them is changed, TiddlyWiki will keep trying to write that file, and other files (which are writable) won't be written as a result.

What's more, the server appears to have accepted the changes, so no error dialog shows up, and [haschanged[]] has no results, so the wiki appears non-dirty. Also, since the changes are cached in Node's memory, reloading the wiki appears to be normal - it's only after you've reloaded the Node daemon that the problem truly becomes apparent.

To Reproduce

Run this script, or its like in the browser:

$ npm install tiddlywiki@latest
$ ./node_modules/.bin/tiddlywiki --version
5.1.22
$ ./node_modules/.bin/tiddlywiki wiki --init server
Copied edition 'server' to wiki
$ ./node_modules/.bin/tiddlywiki wiki --listen port=9091
$ cat tw.log
 syncer-server-filesystem: Dispatching 'save' task: $:/StoryList
Serving on http://127.0.0.1:9091
(press ctrl-C to exit)
$ curl -s -w "%{http_code}\n" -H 'X-Requested-With: TiddlyWiki' -X PUT -d '{"title":"test1","text":"abc123"}' http://localhost:9091/recipes/default/tiddlers/test1
204
$ cat wiki/tiddlers/test1.tid
created: 20200628215555511
modified: 20200628215555511
title: test1

abc123
$ curl -s -w "%{http_code}\n" -H 'X-Requested-With: TiddlyWiki' -X PUT -d '{"title":"test2","text":"def456"}' http://localhost:9091/recipes/default/tiddlers/test2
204
$ cat wiki/tiddlers/test2.tid
created: 20200628215557534
modified: 20200628215557534
title: test2

def456
$ chmod 400 wiki/tiddlers/test1.tid
$ curl -s -w "%{http_code}\n" -H 'X-Requested-With: TiddlyWiki' -X PUT -d '{"title":"test1","text":"abc123\n\nupdate"}' http://localhost:9091/recipes/default/tiddlers/test1
204
$ curl -s -w "%{http_code}\n" -H 'X-Requested-With: TiddlyWiki' -X PUT -d '{"title":"test2","text":"ghi789"}' http://localhost:9091/recipes/default/tiddlers/test2
204
$ cat tw.log
 syncer-server-filesystem: Dispatching 'save' task: $:/StoryList
Serving on http://127.0.0.1:9091
(press ctrl-C to exit)
 syncer-server-filesystem: Dispatching 'save' task: test1
 syncer-server-filesystem: Dispatching 'save' task: test2
 syncer-server-filesystem: Dispatching 'save' task: test1
Sync error while processing save of 'test1': Error: EACCES: permission denied, open '/tmp/tw-test/wiki/tiddlers/test1.tid'
 syncer-server-filesystem: Dispatching 'save' task: test1
Sync error while processing save of 'test1': Error: EACCES: permission denied, open '/tmp/tw-test/wiki/tiddlers/test1.tid'
 syncer-server-filesystem: Dispatching 'save' task: test1
$ cat wiki/tiddlers/test2.tid
created: 20200628215557534
modified: 20200628215557534
title: test2

def456
$ cat wiki/tiddlers/test1.tid
created: 20200628215555511
modified: 20200628215555511
title: test1

abc123

I also reproduced this on Git HEAD (currently 35a842ade).

Expected behavior One broken tiddler shouldn't prevent others from being saved, and the user should be notified of issues saving any tiddlers.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: Arch Linux
• Browser Firefox (but reproduced on the CLI)
• Version: 77.0.1

Additional context

[hoelzro]

I was thinking that the "right" solution for this would be to fail fast - have Node response with a 5xx error so that the user is made readily aware that things didn't work. However, since the saving/syncing/tiddlywebadaptor/filesystem code is so nicely decoupled, only relying on addTiddler and change events, maybe that would be trying to shove a square peg in a round hole, unless an API method were added to the syncer to say "let me know when this change has been successfully persisted by the syncadaptor", or perhaps "let me know when the syncadaptor has stopped saving changes".

Another approach could be to tally the errors and return them during the regular tiddler refresh.

[Jermolene]

Thanks @hoelzro. I agree with the "fail fast" logic, which does indeed imply that we would need a robust way to display server error messages to the browser. That might be as simple as a $:/ServerLog tiddler that is synced in the usual way from server to the client. I guess we'd also need to adjust things so that the log is quiet by default so that the browser can treat a non-empty log as an error.

[hoelzro]

@Jermolene Thinking about it a little more - is there any way we could get PUT /recipes/default/tiddlers/:title to return an error code to the browser in this case? I have a feeling it'll be more difficult to manage, but I can imagine a scenario where a user edits a tiddler, saves that change, sees the cloud icon go from red to normal, and then closes their browser tab with the apparent peace of mind that their work has been saved. I'm not sure how this would look from an architecture perspective, though.

[saqimtiaz]

@Jermolene this of course assumes that we have write access to the tiddler directory, do we check for that during startup? We could also dump the pending data as JSON to a recovery tiddler alongside the $:/ServerLog tiddler, so that the user is made aware of the issue the next time they open the wiki and there is no data loss.

[hoelzro]

@saqimtiaz Definitely a good idea to check for that - we should probably make sure that other potential errors are surfaced in case there are checks we miss at startup, too! I think it would also be a good idea to make sure one tiddler doesn't prevent others from being saved, in case a permission/quota/whatever issue is just holding up a single tiddler.

By the way @Jermolene, I didn't see a test for the filesystem plugin, or for the syncer/saveadaptor mechanism - would you be open to me adding that? If so, what would be the best approach for adding those tests?

[sukima]

Is this only in cases of read only files? I've noticed with my Node based tiddly that changes I've made one day are reverted another day. I presume it is similar to this as an error is never provided but a refresh has missing changes. Maybe my file system has IO issues and that triggers this bug?

[sukima]

I think for me the biggest help would be to return an indicator that it has successfully synced. Once the icon moved from the check mark to a cloud icon I lost the ability to see the status of the saving. If I close my browser before it is able to write everything to the Node server I have no idea. Nothing stops me from destroying changes if they haven't been applied to the Node server wiki.

[Jermolene]

Hi @hoelzro @saqimtiaz @sukima

is there any way we could get PUT /recipes/default/tiddlers/:title to return an error code to the browser in this case?

This seems worth thinking about but would clearly involve a lot of re-engineering, and might require us to tighten the coupling between the tiddlyweb syncadaptor on the browser and the filesystem syncadaptor on the server.

@Jermolene this of course assumes that we have write access to the tiddler directory, do we check for that during startup? We could also dump the pending data as JSON to a recovery tiddler alongside the $:/ServerLog tiddler, so that the user is made aware of the issue the next time they open the wiki and there is no data loss.

I like that idea a lot. Such a failsafe mechanism might give us protection against bugs we don't understand yet.

I didn't see a test for the filesystem plugin, or for the syncer/saveadaptor mechanism - would you be open to me adding that? If so, what would be the best approach for adding those tests?

It would indeed be good to add some tests, but the structure of the code at present might confound those efforts. For example, one would want to be able to instantiate a wiki object and then attach a syncer object to it, but sadly at the moment the syncer and syncadaptor are globals. We might want to be able to mock the file system, too, I don't know how easy that is in Jasmine (I'd be happy to consider moving to a different test runner).

Is this only in cases of read only files? I've noticed with my Node based tiddly that changes I've made one day are reverted another day. I presume it is similar to this as an error is never provided but a refresh has missing changes. Maybe my file system has IO issues and that triggers this bug?

A failed write by Node.js would indeed manifest itself to the user as losing the last edit. In such situations have you seen any error reports in the console?

[sukima]

@Jermolene

have you seen any error reports in the console?

Sadly I don't notice till after the browser closes and reopens the next day. I've been running my Node server via Mac's LaunchControl I will have to investigate a method to capture Node's stdout/stderr.

[hoelzro]

@Jermolene Maybe a good compromise between the current situation and modifying the API would be to have Node continue to return a 204, but only after the new contents have been written to that recovery tiddler that @saqimtiaz proposed.

I'll see what I can do about adding some tests - I don't think I'll be able to leverage the existing Jasmine test plugin, but maybe I just need to think more creatively!

[sukima]

Just an FYI just lost a days worth of work because it said it saved but then a browser reset and all gone. This is really an issue. I pleading for some help. About to downgrade my TW's.

I just downgraded my TW from 5.1.22 to 5.1.21 and my days work of changes came back!!! which means the server had it there all along but for what ever reason the client (when running in 5.1.22) didn't see any of it. This level of inconsistent behavior is very disturbing to me.

[hoelzro]

@sukima I'm wondering if what happened to you was a browser caching issue - does this describe what happened?

• You made a bunch of edits
• Your browser restarted
• You noticed all of your edits were gone :cry:
• You downgraded TiddlyWiki, restarted the Node daemon, and refreshed the tab in your browser open to your wiki
• Your edits returned! :tada:

[sukima]

@hoelzro Almost.

1. Made a bunch of edits
2. browser restarted
3. noticed all of your edits were gone 😢
4. refreshed the page — changes still gone
5. refreshed the page with cache turned off — have a panic attach
6. downgraded TiddlyWiki, restarted the Node daemon
7. refreshed the same tab
8. Your edits returned! 🎉

🤷 Sorry this is probably an unrelated red herring.

[saqimtiaz]

@joshuafontany would you be so kind as to read the original post and reflect on how the recent changes to saving mechanism might ameliorate the problem described here? Thank you.

[joshuafontany]

@saqimtiaz I was already lurking here while coding the fix. :) Sorry, should have commented. The new changes fix this. Any time a write fails, the tiddler's file path is run thru encodeURIComponent() and then saved to the primary $tw.wikiTiddlersPath (with auto incrementing #s if that file already exists from another tiddler).

[saqimtiaz]

@joshuafontany that is great, thank you.

We should however leave this issue open. There is still a need to sync back to the client a tiddler/message indicating that there was a problem with saving.

[Jermolene]

We should however leave this issue open. There is still a need to sync back to the client a tiddler/message indicating that there was a problem with saving.

We may be able to use the alert mechanism here. In principle, we could sync $:/tags/Alert tiddlers to particular clients whenever we want to display an error message.

[saqimtiaz]

@Jermolene that would work. The failure condition at which @joshuafontany's code writes out the tiddlers with file path after encoding with encoreURIComponent, would be a good flag to create the alert tiddlers.

Suggest we take a look at this after 5.1.23.

[joshuafontany]

Commenting so I remember to come back and review this. :)