Do not disable shipped apps on exceptions

@rullzer

having redis set up
having shares
having a network hiccup
once the connection to the redis server is somehow not working, the server catches the exception and will disable the app in which it happened - in this case it was files_sharing which caused all shares being deleted from user computers :/

cc @rullzer @schiessle @blizzz @ChristophWurst @nickvergessen @danxuliu Do you have an idea how to mitigate this?

The error looked like this:

{
	"Exception":"RedisException",
	"Message":"read error on connection",
	"Code":0,
	"Trace":"
		#0 lib/private/Memcache/Redis.php(52): Redis->get('686f2401e81421f...')
		#1 apps/user_ldap/lib/Connection.php(227): OC\\Memcache\\Redis->get('LDAP-user_ldap-...')
		#2 apps/user_ldap/lib/Group_LDAP.php(609): OCA\\User_LDAP\\Connection->getFromCache('LDAP-user_ldap-...')
		#3 apps/user_ldap/lib/Group_Proxy.php(122): OCA\\User_LDAP\\Group_LDAP->getUserGroups('130b2c0c-4092-1...')
		#4 lib/private/Group/Manager.php(269): OCA\\User_LDAP\\Group_Proxy->getUserGroups('130b2c0c-4092-1...')
		#5 lib/private/Group/Manager.php(256): OC\\Group\\Manager->getUserIdGroups('130b2c0c-4092-1...')
		#6 lib/private/Share20/DefaultShareProvider.php(710): OC\\Group\\Manager->getUserGroups(Object(OC\\User\\User))
		#7 lib/private/Share20/Manager.php(1121): OC\\Share20\\DefaultShareProvider->getSharedWith('130b2c0c-4092-1...', 1, NULL, -1, 0)
		#8 apps/files_sharing/lib/MountProvider.php(73): OC\\Share20\\Manager->getSharedWith('130b2c0c-4092-1...', 1, NULL, -1)
		#9 lib/private/Files/Config/MountProviderCollection.php(108): OCA\\Files_Sharing\\MountProvider->getMountsForUser(Object(OC\\User\\User), Object(OC\\Files\\Storage\\StorageFactory))
		#10 lib/private/Files/Filesystem.php(445): OC\\Files\\Config\\MountProviderCollection->addMountForUser(Object(OC\\User\\User), Object(OC\\Files\\Mount\\Manager))
		#11 lib/private/Files/Filesystem.php(374): OC\\Files\\Filesystem::initMountPoints('130b2c0c-4092-1...')
		#12 lib/private/legacy/util.php(280): OC\\Files\\Filesystem::init('130b2c0c-4092-1...', '/130b2c0c-4092-...')
		#13 apps/dav/lib/Connector/Sabre/Auth.php(123): OC_Util::setupFS('130b2c0c-4092-1...')
		#14 3rdparty/sabre/dav/lib/DAV/Auth/Backend/AbstractBasic.php(105): OCA\\DAV\\Connector\\Sabre\\Auth->validateUserPass(*** sensitive parameters replaced ***)
		#15 apps/dav/lib/Connector/Sabre/Auth.php(252): Sabre\\DAV\\Auth\\Backend\\AbstractBasic->check(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
		#16 apps/dav/lib/Connector/Sabre/Auth.php(154): OCA\\DAV\\Connector\\Sabre\\Auth->auth(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
		#17 3rdparty/sabre/dav/lib/DAV/Auth/Plugin.php(201): OCA\\DAV\\Connector\\Sabre\\Auth->check(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
		#18 3rdparty/sabre/dav/lib/DAV/Auth/Plugin.php(150): Sabre\\DAV\\Auth\\Plugin->check(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
		#19 [internal function]: Sabre\\DAV\\Auth\\Plugin->beforeMethod(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
		#20 3rdparty/sabre/event/lib/EventEmitterTrait.php(105): call_user_func_array(Array, Array)
		#21 3rdparty/sabre/dav/lib/DAV/Server.php(466): Sabre\\Event\\EventEmitter->emit('beforeMethod', Array)
		#22 3rdparty/sabre/dav/lib/DAV/Server.php(254): Sabre\\DAV\\Server->invokeMethod(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
		#23 apps/dav/appinfo/v1/webdav.php(76): Sabre\\DAV\\Server->exec()
		#24 remote.php(162): require_once('/var/www/cloud/...')
		#25 {main}",
	"File":"lib/private/Memcache/Redis.php",
	"Line":52
}

server/lib/private/legacy/app.php

Lines 237 to 243 in e3c0fd6

    
           } catch (Error $ex) { 
        
           	\OC::$server->getLogger()->logException($ex); 
        
           	$blacklist = \OC::$server->getAppManager()->getAlwaysEnabledApps(); 
        
           	if (!in_array($app, $blacklist)) { 
        
           		self::disable($app); 
        
           	} 
        
           }

Check the exception for some types?

Or just change getAlwaysEnabledApps to getShippedApps

using getShippedApps instead could improve it, but it won't catch all scenarios. After all, any other app could also cause similar affects, when Redis just becomes unavailable.

Additionally we could whitelist some known Exceptions (e.g. also databases if they're down). It's not super proof still.

Since the last upgrade (12.0.4) following apps were disabled several times and i believe it's the same issue:

files_sharing
files_trashbin

After all, any other app could also cause similar affects, when Redis just becomes unavailable.

Yeah, but than again. If the app fails with a database error, we can't know was it because the database had a hick up, or is the app broken and using an invalid schema etc. I'd say lets start this slowly. I will make a patch to protect shipped apps.

Navigation Menu

Repository navigation

Do not disable shipped apps on exceptions #7865

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	} catch (Error $ex) {
	\OC::$server->getLogger()->logException($ex);
	$blacklist = \OC::$server->getAppManager()->getAlwaysEnabledApps();
	if (!in_array($app, $blacklist)) {
	self::disable($app);
	}
	}

Do not disable shipped apps on exceptions #7865

Description

Activity

MorrisJobke commented on Jan 15, 2018

nickvergessen commented on Jan 15, 2018

nickvergessen commented on Jan 15, 2018

blizzz commented on Jan 15, 2018

runout-at commented on Jan 15, 2018

nickvergessen commented on Jan 16, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions