Do not disable shipped apps on exceptions

[MorrisJobke]

• 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?

[MorrisJobke]

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
}

[nickvergessen]

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?

[nickvergessen]

Or just change getAlwaysEnabledApps to getShippedApps

[blizzz]

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.

[runout-at]

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

[nickvergessen]

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.