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

Fix for previews not being generated sometimes #41067

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

akhil1508
Copy link
Contributor

@akhil1508 akhil1508 commented Oct 23, 2023

Summary

  • A lot of unnecessary rmdir calls are done on the preview directory

  • After investigating, here are the findings to justify the fix:

    • As per
      public function getId() {
      there is a return type of int or null
    • A null return by this method call over here makes sure that an empty string is passed to $this->appData->getFolder()
    • As per the logic, for an empty string this will return the preview folder i.e. appdata_*****/preview in our case
    • There is then an attempt to delete this entire folder
  • On our instance we did find logs that a delete is attempted on the entire preview folder as per [Bug]: Preview for some photos not generated #36463 (comment)

  • Further, by logging I could confirm that $node->getId() is indeed null in one case leading to consistent triggering of this rmdir

  • One case is the upload of a partial file (.part extension)

    • The Nextcloud clients avoid this but certain DAV clients allow to upload to nextcloud servers
    • This kind of file is ignored by isPartialFile
    • In this example a file with the .part extension succesfully triggers an rmdir of the entire preview directory

Stack trace of the bug

{
	"reqId": "*****",
	"level": 3,
	"time": "2023-10-17T17:50:58+00:00",
	"remoteAddr": "***.***.***.***",
	"user": "*****",
	"app": "PHP",
	"method": "MOVE",
	"url": "/remote.php/dav/uploads/*****/*****/*****",
	"message": "rmdir(/var/www/data/appdata_*****/preview/1): Directory not empty at /var/www/html/lib/private/Files/Storage/Local.php#139",
	"version": "25.0.8.2",
	"exception": {
		"Exception": "Error",
		"Message": "rmdir(/var/www/data/appdata_*****/preview/1): Directory not empty at /var/www/html/lib/private/Files/Storage/Local.php#139",
		"Code": 0,
		"Trace": [
			{
				"function": "onError",
				"class": "OC\\Log\\ErrorHandler",
				"type": "::",
				"args": [
					2,
					"rmdir(/var/www/data/appdata_*****/preview/1): Directory not empty",
					"/var/www/html/lib/private/Files/Storage/Local.php",
					139
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/Storage/Local.php",
				"line": 139,
				"function": "rmdir",
				"args": [
					"/var/www/data/appdata_*****/preview/1"
				]
			},
			{
				"function": "rmdir",
				"class": "OC\\Files\\Storage\\Local",
				"type": "->",
				"args": [
					"appdata_*****/preview"
				]
			},
			{
				"file": "/var/www/html/apps/files_trashbin/lib/Storage.php",
				"line": 193,
				"function": "call_user_func",
				"args": [
					[
						[
							"OC\\Files\\Storage\\LocalRootStorage"
						],
						"rmdir"
					],
					"appdata_*****/preview"
				]
			},
			{
				"file": "/var/www/html/apps/files_trashbin/lib/Storage.php",
				"line": 125,
				"function": "doDelete",
				"class": "OCA\\Files_Trashbin\\Storage",
				"type": "->",
				"args": [
					"appdata_*****/preview",
					"rmdir"
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/View.php",
				"line": 1181,
				"function": "rmdir",
				"class": "OCA\\Files_Trashbin\\Storage",
				"type": "->",
				"args": [
					"appdata_*****/preview"
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/View.php",
				"line": 349,
				"function": "basicOperation",
				"class": "OC\\Files\\View",
				"type": "->",
				"args": [
					"rmdir",
					"/appdata_*****/preview",
					[
						"delete"
					]
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/Node/Folder.php",
				"line": 363,
				"function": "rmdir",
				"class": "OC\\Files\\View",
				"type": "->",
				"args": [
					"/appdata_*****/preview"
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/SimpleFS/SimpleFolder.php",
				"line": 68,
				"function": "delete",
				"class": "OC\\Files\\Node\\Folder",
				"type": "->",
				"args": []
			},
			{
				"file": "/var/www/html/lib/private/Preview/Watcher.php",
				"line": 65,
				"function": "delete",
				"class": "OC\\Files\\SimpleFS\\SimpleFolder",
				"type": "->",
				"args": []
			},
			{
				"file": "/var/www/html/lib/private/Preview/Watcher.php",
				"line": 54,
				"function": "deleteNode",
				"class": "OC\\Preview\\Watcher",
				"type": "->",
				"args": [
					"*** sensitive parameters replaced ***"
				]
			},
			{
				"file": "/var/www/html/lib/private/Preview/WatcherConnector.php",
				"line": 63,
				"function": "postWrite",
				"class": "OC\\Preview\\Watcher",
				"type": "->",
				"args": [
					"*** sensitive parameters replaced ***"
				]
			},
			{
				"function": "OC\\Preview\\{closure}",
				"class": "OC\\Preview\\WatcherConnector",
				"type": "->",
				"args": [
					"*** sensitive parameters replaced ***"
				]
			},
			{
				"file": "/var/www/html/lib/private/Hooks/EmitterTrait.php",
				"line": 106,
				"function": "call_user_func_array",
				"args": [
					[
						"Closure"
					],
					[
						"*** sensitive parameters replaced ***"
					]
				]
			},
			{
				"file": "/var/www/html/lib/private/Hooks/PublicEmitter.php",
				"line": 40,
				"function": "emit",
				"class": "OC\\Hooks\\BasicEmitter",
				"type": "->",
				"args": [
					"\\OC\\Files",
					"postWrite",
					[
						"*** sensitive parameters replaced ***"
					]
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/Node/Root.php",
				"line": 143,
				"function": "emit",
				"class": "OC\\Hooks\\PublicEmitter",
				"type": "->",
				"args": [
					"\\OC\\Files",
					"postWrite",
					[
						"*** sensitive parameters replaced ***"
					]
				]
			},
			{
				"function": "emit",
				"class": "OC\\Files\\Node\\Root",
				"type": "->",
				"args": [
					"\\OC\\Files",
					"postWrite",
					[
						"*** sensitive parameters replaced ***"
					]
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/Node/LazyFolder.php",
				"line": 72,
				"function": "call_user_func_array",
				"args": [
					[
						[
							"OC\\Files\\Node\\Root"
						],
						"emit"
					],
					[
						"\\OC\\Files",
						"postWrite",
						[
							"*** sensitive parameters replaced ***"
						]
					]
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/Node/LazyFolder.php",
				"line": 100,
				"function": "__call",
				"class": "OC\\Files\\Node\\LazyFolder",
				"type": "->",
				"args": [
					"emit",
					[
						"\\OC\\Files",
						"postWrite",
						[
							"*** sensitive parameters replaced ***"
						]
					]
				]
			},
			{
				"file": "/var/www/html/lib/private/Files/Node/HookConnector.php",
				"line": 117,
				"function": "emit",
				"class": "OC\\Files\\Node\\LazyFolder",
				"type": "->",
				"args": [
					"\\OC\\Files",
					"postWrite",
					[
						"*** sensitive parameters replaced ***"
					]
				]
			},
			{
				"file": "/var/www/html/lib/private/legacy/OC_Hook.php",
				"line": 106,
				"function": "postWrite",
				"class": "OC\\Files\\Node\\HookConnector",
				"type": "->",
				"args": [
					[
						"/****/*****/*****/*****.part"
					]
				]
			},
			{
				"file": "/var/www/html/apps/dav/lib/Connector/Sabre/File.php",
				"line": 473,
				"function": "emit",
				"class": "OC_Hook",
				"type": "::",
				"args": [
					"OC_Filesystem",
					"post_write",
					[
						"/****/*****/*****/*****.part"
					]
				]
			},
			{
				"file": "/var/www/html/apps/dav/lib/Connector/Sabre/File.php",
				"line": 398,
				"function": "emitPostHooks",
				"class": "OCA\\DAV\\Connector\\Sabre\\File",
				"type": "->",
				"args": [
					true
				]
			},
			{
				"file": "/var/www/html/apps/dav/lib/Connector/Sabre/Directory.php",
				"line": 151,
				"function": "put",
				"class": "OCA\\DAV\\Connector\\Sabre\\File",
				"type": "->",
				"args": [
					null
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/dav/lib/DAV/Tree.php",
				"line": 307,
				"function": "createFile",
				"class": "OCA\\DAV\\Connector\\Sabre\\Directory",
				"type": "->",
				"args": [
					"*****",
					null
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/dav/lib/DAV/Tree.php",
				"line": 133,
				"function": "copyNode",
				"class": "Sabre\\DAV\\Tree",
				"type": "->",
				"args": [
					[
						"OCA\\DAV\\Upload\\FutureFile"
					],
					[
						"OCA\\DAV\\Connector\\Sabre\\Directory"
					],
					"*****"
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/dav/lib/DAV/Tree.php",
				"line": 163,
				"function": "copy",
				"class": "Sabre\\DAV\\Tree",
				"type": "->",
				"args": [
					"uploads/*****/*****/*****",
					"files/*****/****/*****/*****/*****.part"
				]
			},
			{
				"file": "/var/www/html/apps/dav/lib/Upload/ChunkingPlugin.php",
				"line": 94,
				"function": "move",
				"class": "Sabre\\DAV\\Tree",
				"type": "->",
				"args": [
					"uploads/*****/*****/*****",
					"files/*****/****/*****/*****/*****.part"
				]
			},
			{
				"file": "/var/www/html/apps/dav/lib/Upload/ChunkingPlugin.php",
				"line": 76,
				"function": "performMove",
				"class": "OCA\\DAV\\Upload\\ChunkingPlugin",
				"type": "->",
				"args": [
					"uploads/*****/*****/*****",
					"files/*****/****/*****/*****/*****.part"
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
				"line": 89,
				"function": "beforeMove",
				"class": "OCA\\DAV\\Upload\\ChunkingPlugin",
				"type": "->",
				"args": [
					"uploads/*****/*****/*****",
					"files/*****/****/*****/*****/*****.part"
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/dav/lib/DAV/CorePlugin.php",
				"line": 603,
				"function": "emit",
				"class": "Sabre\\DAV\\Server",
				"type": "->",
				"args": [
					"beforeMove",
					[
						"uploads/*****/*****/*****",
						"files/*****/****/*****/*****/*****.part"
					]
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
				"line": 89,
				"function": "httpMove",
				"class": "Sabre\\DAV\\CorePlugin",
				"type": "->",
				"args": [
					[
						"Sabre\\HTTP\\Request"
					],
					[
						"Sabre\\HTTP\\Response"
					]
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php",
				"line": 472,
				"function": "emit",
				"class": "Sabre\\DAV\\Server",
				"type": "->",
				"args": [
					"method:MOVE",
					[
						[
							"Sabre\\HTTP\\Request"
						],
						[
							"Sabre\\HTTP\\Response"
						]
					]
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php",
				"line": 253,
				"function": "invokeMethod",
				"class": "Sabre\\DAV\\Server",
				"type": "->",
				"args": [
					[
						"Sabre\\HTTP\\Request"
					],
					[
						"Sabre\\HTTP\\Response"
					]
				]
			},
			{
				"file": "/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php",
				"line": 321,
				"function": "start",
				"class": "Sabre\\DAV\\Server",
				"type": "->",
				"args": []
			},
			{
				"file": "/var/www/html/apps/dav/lib/Server.php",
				"line": 360,
				"function": "exec",
				"class": "Sabre\\DAV\\Server",
				"type": "->",
				"args": []
			},
			{
				"file": "/var/www/html/apps/dav/appinfo/v2/remote.php",
				"line": 35,
				"function": "exec",
				"class": "OCA\\DAV\\Server",
				"type": "->",
				"args": []
			},
			{
				"file": "/var/www/html/remote.php",
				"line": 172,
				"args": [
					"/var/www/html/apps/dav/appinfo/v2/remote.php"
				],
				"function": "require_once"
			}
		],
		"File": "/var/www/html/lib/private/Log/ErrorHandler.php",
		"Line": 92,
		"CustomMessage": "--"
	}
}

Checklist

@akhil1508 akhil1508 changed the title Check if the node has a null ID before deleting in preview watcher Fix for previews not being generated sometimes Oct 23, 2023
lib/private/Preview/Watcher.php Show resolved Hide resolved
@solracsf solracsf added this to the Nextcloud 28 milestone Oct 27, 2023
@tcitworld tcitworld requested review from a team, icewind1991, nfebe and sorbaugh and removed request for a team October 30, 2023 08:10
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't hurt!

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@come-nc IIRC, you looked into the null file id on some file events. Is it similar?

@come-nc
Copy link
Contributor

come-nc commented Nov 6, 2023

@come-nc IIRC, you looked into the null file id on some file events. Is it similar?

Not really, here it’s expected for the node to have no id since it’s new, I think. In the cases I looked into it was paths outside the user directory which ended up trigerring events on null paths.

This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@blizzz blizzz merged commit c4f4c5a into nextcloud:master Jan 18, 2024
34 of 39 checks passed
@SystemKeeper
Copy link
Contributor

@blizzz backport? 🤔

@blizzz
Copy link
Member

blizzz commented Jan 19, 2024

I understand it was first discovered on 25, so since then everything is affected.

@blizzz
Copy link
Member

blizzz commented Jan 19, 2024

/skjnldsv-backport to stable28

@blizzz
Copy link
Member

blizzz commented Jan 19, 2024

/skjnldsv-backport to stable27

@blizzz
Copy link
Member

blizzz commented Jan 19, 2024

/skjnldsv-backport to stable26

@susnux
Copy link
Contributor

susnux commented Jan 20, 2024

/backport to stable28

@SystemKeeper
Copy link
Contributor

Hm, does backporting not work if it’s from a fork?

@susnux
Copy link
Contributor

susnux commented Jan 22, 2024

Hm, does backporting not work if it’s from a fork?

cc @skjnldsv

@popos123
Copy link

popos123 commented Jun 3, 2024

hi, i see that fix was implemented in version 29.0.1? becouse i have this bug on the nevest version too
image

@chrisborell
Copy link

Still occuring for me in [Nextcloud Hub 8] (29.0.2)

Screenshot 2024-06-20 at 10 09 39 am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Preview for some photos not generated