Never log failure to retrieve an optional property

[aleasto]

Chromium and Electron apps do not implement the IconAccessibleDesc property and reply to the Get() method with a generic
org.freedesktop.DBus.Error.Failed

Rather than adding the generic error to the list of allowed errors, remove the filtering on the error type altogether.

Fixes: #534
LP: #2064698

[3v1n0]

Mh, I was thinking on whether doing this for a long time, but I always decided not to do it, since this is not an extension bug but rather applications issue that implement poorly the protocol. That's flawed for many reasons anyways, but that's not an excuse not to be compliant with basic dbus expectations.

Also I'm still curious why people is bothered by journal debug infos :-D, however if there's much request I can re-consider the decision.

Rather than adding the generic error to the list of allowed errors, remove the filtering on the error type altogether.

On this, though, I think it's still better to filter the generic error instead, because at least would allow us not to ignore the non-Gio.DBusError errors. So if there's some other JS error in the stack, we don't miss it (and it's easy to happen on new shell versions or syntax changes).

[aleasto]

Also I'm still curious why people is bothered by journal debug infos :-D, however if there's much request I can re-consider the decision.

It's a full stack trace, it takes up something like 80% of my journal because it's logged multiple times a minute... It is also logged at LOG_ERR priority which makes it more than debug infos

On this, though, I think it's still better to filter the generic error instead, because at least would allow us not to ignore the non-Gio.DBusError errors. So if there's some other JS error in the stack, we don't miss it (and it's easy to happen on new shell versions or syntax changes).

Good point. How about this catch-all instanceof Gio.DBusError?

[aleasto]

applications issue that implement poorly the protocol

BTW we can totally send a patch to chromium, but it wouldn't start rolling into Electron apps before 2026 🤣

[3v1n0]

applications issue that implement poorly the protocol

BTW we can totally send a patch to chromium, but it wouldn't start rolling into Electron apps before 2026 🤣

Ahah, well... If you've some spare time, it wouldn't too bad to fix electron, but in the mean time that's fine to do what you suggest.

Good point. How about this catch-all instanceof Gio.DBusError?

Yeah, you can filter by using error.matches(...), also maybe in that case you can avoid using logError and instead rely on Logger.debug, printing both the error.message (or is ${error} (.e.g. error.toString() more informative actually)? and the stack trace.

So that we can still see the results when G_MESSAGES_DEBUG="Ubuntu AppIndicators" or `G_MESSAGES_DEBUG=all are set.

[aleasto]

Ah actually refreshProperty() itself already logs the message.

gnome-shell-extension-appindicator/appIndicator.js

Lines 314 to 322 in 19ee5b0

	// the property may not even exist, silently ignore it
	Util.Logger.debug(`Error when calling 'Get(${propertyName})' ` +
	`in ${this.gName}, ${this.gObjectPath}, ` +
	`org.freedesktop.DBus.Properties, ${this.gInterfaceName} ` +
	`while refreshing property ${propertyName}: ${e}`);
	this.set_cached_property(propertyName, null);
	this._cancellables.delete(propertyName);
	delete this._changedProperties[propertyName];
	throw e;

Weird that you say "silently ignore it" and then throw 🤔
So here we only need to catch and actually ignore it, which the current diff already does.

[3v1n0]

Weird that you say "silently ignore it" and then throw 🤔

throwing is needed to ensure that the error is propagated, then it's up to the users of such function to handle it or now, and now we've decided to fully ignore it, but it's something that is always better to do in a leaf of a call stack (so that we don't miss things).