Player error handling

Add an `error` event to the Player so users can handle DBus connection errors. Proxy dbus connection and export errors through this event. Update dbus-next to 0.4 and use the new interface export api. Update tests for the new features and add a `ping` fixture. fixes dbusjs#32
acrisci · Mar 25, 2019 · 4eace3d · 4eace3d
1 parent eee6744
commit 4eace3d
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 40 deletions.
diff --git a/package.json b/package.json
@@ -27,7 +27,7 @@
   },
   "homepage": "https://github.com/emersion/mpris-service",
   "dependencies": {
-    "dbus-next": "^0.3.2",
+    "dbus-next": "^0.4.1",
     "deep-equal": "^1.0.1",
     "source-map-support": "^0.5.9"
   },

diff --git a/src/index.js b/src/index.js
@@ -49,6 +49,10 @@ function lcfirst(str) {
  * * `loopStatus` - Sets the loop status of the player to either 'None', 'Track', or 'Playlist'. With event data `loopStatus`.
  * * `activatePlaylist` -  Starts playing the given playlist. With event data `playlistId`.
  *
+ * The Player may also emit an `error` event with the underlying Node `Error`
+ * as the event data. After receiving this event, the Player may be
+ * disconnected.
+ *
  * ```
  * player.on('play', () => {
  *   realPlayer.play();
@@ -173,11 +177,16 @@ function Player(opts) {
 util.inherits(Player, events.EventEmitter);
 
 Player.prototype.init = function(opts) {
+  let that = this;
   this.serviceName = `org.mpris.MediaPlayer2.${this.name}`;
   dbus.validators.assertBusNameValid(this.serviceName);
 
   this._bus = dbus.sessionBus();
 
+  this._bus.connection.on('error', (err) => {
+    that.emit('error', err);
+  });
+
   this.interfaces = {};
 
   this._addRootInterface(this._bus, opts);
@@ -191,6 +200,17 @@ Player.prototype.init = function(opts) {
   if (this.supportedInterfaces.indexOf('playlists') >= 0) {
     this._addPlaylistsInterface(this._bus);
   }
+
+  this._bus.requestName(this.serviceName)
+    .then((name) => {
+      for (let k of Object.keys(that.interfaces)) {
+        let iface = that.interfaces[k];
+        name.export(MPRIS_PATH, iface);
+      }
+    })
+    .catch((err) => {
+      that.emit('error', err);
+    });
 };
 
 Player.prototype._addRootInterface = function(bus, opts) {
@@ -199,7 +219,6 @@ Player.prototype._addRootInterface = function(bus, opts) {
     ['Identity', 'Fullscreen', 'SupportedUriSchemes', 'SupportedMimeTypes',
     'CanQuit', 'CanRaise', 'CanSetFullscreen', 'HasTrackList',
     'DesktopEntry']);
-  bus.export(this.serviceName, MPRIS_PATH, this.interfaces.root);
 };
 
 Player.prototype._addPlayerInterface = function(bus) {
@@ -208,7 +227,6 @@ Player.prototype._addPlayerInterface = function(bus) {
     'Metadata', 'Volume', 'CanControl', 'CanPause', 'CanPlay', 'CanSeek',
     'CanGoNext', 'CanGoPrevious', 'MinimumRate', 'MaximumRate'];
   this._addEventedPropertiesList(this.interfaces.player, eventedProps);
-  bus.export(this.serviceName, MPRIS_PATH, this.interfaces.player);
 };
 
 Player.prototype._addTracklistInterface = function(bus) {
@@ -226,15 +244,12 @@ Player.prototype._addTracklistInterface = function(bus) {
     enumerable: true,
     configurable: true
   });
-
-  bus.export(this.serviceName, MPRIS_PATH, this.interfaces.tracklist);
 };
 
 Player.prototype._addPlaylistsInterface = function(bus) {
   this.interfaces.playlists = new PlaylistsInterface(this);
   this._addEventedPropertiesList(this.interfaces.playlists,
     ['PlaylistCount', 'ActivePlaylist']);
-  bus.export(this.serviceName, MPRIS_PATH, this.interfaces.playlists);
 }
 
 /**

diff --git a/test/fixtures.js b/test/fixtures.js
@@ -0,0 +1,5 @@
+module.exports.ping = async function(bus) {
+  // this will call dbus to introspect this object doing a round trip
+  await bus.getProxyObject('org.freedesktop.DBus', '/org/freedesktop/DBus');
+  return true;
+}
diff --git a/test/player.test.js b/test/player.test.js
@@ -2,8 +2,9 @@ const dbus = require('dbus-next');
 const Variant = dbus.Variant;
 const Player = require('../dist');
 const JSBI = require('jsbi');
+let { ping } = require('./fixtures');
 
-const DBusError = dbus.interface.DBusError;
+const DBusError = dbus.DBusError;
 
 const ROOT_IFACE = 'org.mpris.MediaPlayer2';
 const PLAYER_IFACE = 'org.mpris.MediaPlayer2.Player';
@@ -22,6 +23,10 @@ var player = Player({
 
 let bus = dbus.sessionBus();
 
+beforeAll(async () => {
+  return await ping(bus);
+});
+
 afterAll(() => {
   player._bus.connection.stream.end();
   bus.connection.stream.end();
@@ -69,12 +74,6 @@ test('calling the player methods on the bus emits the signals on the object', as
 
 test('getting and setting properties on the player and on the interface should work', async () => {
   let obj = await bus.getProxyObject('org.mpris.MediaPlayer2.playertest', '/org/mpris/MediaPlayer2');
-  let dbusObj = await bus.getProxyObject('org.freedesktop.DBus', '/org/freedesktop/DBus');
-
-  let ping = async () => {
-    return dbusObj.getInterface('org.freedesktop.DBus').ListNames();
-  };
-
   let playerIface = obj.getInterface(PLAYER_IFACE);
   let props = obj.getInterface('org.freedesktop.DBus.Properties');
 
@@ -86,7 +85,7 @@ test('getting and setting properties on the player and on the interface should w
     'xesam:artist': ['Katy Perry'],
     'xesam:title': 'Rise'
   };
-  await ping();
+  await ping(bus);
   let changed = {
     Metadata: new Variant('a{sv}', {
       'xesam:artist': new Variant('as', ['Katy Perry']),
@@ -101,12 +100,12 @@ test('getting and setting properties on the player and on the interface should w
   // setting the metadata again to the same thing should only emit
   // PropertiesChanged once
   player.metadata = JSON.parse(JSON.stringify(player.metadata));
-  await ping();
+  await ping(bus);
   expect(cb).toHaveBeenCalledTimes(1);
 
   // PlaybackStatus
   player.playbackStatus = Player.PLAYBACK_STATUS_PAUSED;
-  await ping();
+  await ping(bus);
   changed = {
     PlaybackStatus: new Variant('s', 'Paused')
   };
@@ -116,7 +115,7 @@ test('getting and setting properties on the player and on the interface should w
 
   // LoopStatus
   player.loopStatus = Player.LOOP_STATUS_TRACK;
-  await ping();
+  await ping(bus);
   changed = {
     LoopStatus: new Variant('s', 'Track')
   };
@@ -146,7 +145,7 @@ test('getting and setting properties on the player and on the interface should w
   for (let name of doubleProps) {
     let playerName = lcFirst(name);
     player[playerName] = 0.05;
-    await ping();
+    await ping(bus);
     changed = {};
     changed[name] = new Variant('d', 0.05);
     expect(cb).toHaveBeenLastCalledWith(PLAYER_IFACE, changed, []);
@@ -174,7 +173,7 @@ test('getting and setting properties on the player and on the interface should w
     let playerName = lcFirst(name);
     let newValue = !player[playerName];
     player[playerName] = newValue;
-    await ping();
+    await ping(bus);
     changed = {};
     changed[name] = new Variant('b', newValue);
     expect(cb).toHaveBeenLastCalledWith(PLAYER_IFACE, changed, []);
@@ -190,21 +189,17 @@ test('getting and setting properties on the player and on the interface should w
       await props.Set(PLAYER_IFACE, name, new Variant('b', nextNewValue));
       expect(playerCb).toHaveBeenCalledWith(nextNewValue);
       expect(player[playerName]).toEqual(nextNewValue);
+      await ping(bus);
     }
   }
 });
 
 test('position specific properties, methods, and signals should work', async () => {
   // note: they are responsible for setting the position, not the methods directly
   let obj = await bus.getProxyObject('org.mpris.MediaPlayer2.playertest', '/org/mpris/MediaPlayer2');
-  let dbusObj = await bus.getProxyObject('org.freedesktop.DBus', '/org/freedesktop/DBus');
   let playerIface = obj.getInterface(PLAYER_IFACE);
   let props = obj.getInterface('org.freedesktop.DBus.Properties');
 
-  let ping = async () => {
-    return dbusObj.getInterface('org.freedesktop.DBus').ListNames();
-  };
-
   // position defaults to always being 0
   let position = await props.Get(PLAYER_IFACE, 'Position');
   expect(position).toEqual(new Variant('x', JSBI.BigInt(0)));
@@ -232,6 +227,6 @@ test('position specific properties, methods, and signals should work', async ()
   cb = jest.fn();
   playerIface.once('Seeked', cb);
   player.seeked(200);
-  await ping();
+  await ping(bus);
   expect(cb).toHaveBeenCalledWith(JSBI.BigInt(200));
 });
diff --git a/test/playlists.test.js b/test/playlists.test.js
@@ -2,6 +2,7 @@ let dbus = require('dbus-next');
 let Variant = dbus.Variant;
 let Player = require('../dist');
 let JSBI = require('jsbi');
+let { ping } = require('./fixtures');
 
 const ROOT_IFACE = 'org.mpris.MediaPlayer2';
 const PLAYER_IFACE = 'org.mpris.MediaPlayer2.Player';
@@ -66,12 +67,8 @@ test('default state of the playlists interface', async () => {
 
 test('setting a playlist on the player works', async () => {
   let obj = await bus.getProxyObject('org.mpris.MediaPlayer2.playliststest', '/org/mpris/MediaPlayer2');
-  let dbusObj = await bus.getProxyObject('org.freedesktop.DBus', '/org/freedesktop/DBus');
   let playlistsIface = obj.getInterface(PLAYLISTS_IFACE);
   let props = obj.getInterface('org.freedesktop.DBus.Properties');
-  let ping = async () => {
-    return dbusObj.getInterface('org.freedesktop.DBus').ListNames();
-  };
 
   let propsCb = jest.fn();
   props.on('PropertiesChanged', propsCb);
@@ -102,7 +99,7 @@ test('setting a playlist on the player works', async () => {
     }
   ]);
 
-  await ping();
+  await ping(bus);
 
   expect(propsCb).toHaveBeenCalledWith(PLAYLISTS_IFACE, {
     PlaylistCount: new Variant('u', 4)
@@ -118,7 +115,7 @@ test('setting a playlist on the player works', async () => {
 
   player.setActivePlaylist(player.playlists[1].Id);
 
-  await ping();
+  await ping(bus);
 
   let expectedActivePlaylist = new Variant('(b(oss))',
     [

diff --git a/test/root.test.js b/test/root.test.js
@@ -1,6 +1,7 @@
 let dbus = require('dbus-next');
 let Variant = dbus.Variant;
 let Player = require('../dist');
+let { ping } = require('./fixtures');
 
 const ROOT_IFACE = 'org.mpris.MediaPlayer2';
 const PLAYER_IFACE = 'org.mpris.MediaPlayer2.Player';
@@ -19,6 +20,10 @@ var player = Player({
 
 let bus = dbus.sessionBus();
 
+beforeAll(async () => {
+  return await ping(bus);
+});
+
 afterAll(() => {
   player._bus.connection.stream.end();
   bus.connection.stream.end();
@@ -37,17 +42,13 @@ test('calling methods should raise a signal on the player', async () => {
   player.once('raise', cb);
   await root.Raise();
   expect(cb).toHaveBeenCalledWith();
+  await ping(bus);
 });
 
 test('setting properties on the player should show up on dbus and raise a signal', async () => {
   let obj = await bus.getProxyObject('org.mpris.MediaPlayer2.roottest', '/org/mpris/MediaPlayer2');
   let root = obj.getInterface(ROOT_IFACE);
   let props = obj.getInterface('org.freedesktop.DBus.Properties');
-  let dbusObj = await bus.getProxyObject('org.freedesktop.DBus', '/org/freedesktop/DBus');
-
-  let ping = async () => {
-    return dbusObj.getInterface('org.freedesktop.DBus').ListNames();
-  };
 
   let cb = jest.fn();
   props.on('PropertiesChanged', cb);
@@ -60,7 +61,7 @@ test('setting properties on the player should show up on dbus and raise a signal
     expect(gotten).toEqual(new Variant('as', player[playerName]));
     let newValue = ['foo', 'bar'];
     player[playerName] = newValue;
-    await ping();
+    await ping(bus);
     let changed = {};
     changed[name] = new Variant('as', newValue);
     expect(cb).toHaveBeenLastCalledWith(ROOT_IFACE, changed, []);
@@ -72,7 +73,7 @@ test('setting properties on the player should show up on dbus and raise a signal
     let playerName = lcFirst(name);
     let newValue = !player[playerName];
     player[playerName] = newValue;
-    await ping();
+    await ping(bus);
     changed = {};
     changed[name] = new Variant('b', newValue);
     expect(cb).toHaveBeenCalledWith(ROOT_IFACE, changed, []);
@@ -86,7 +87,7 @@ test('setting properties on the player should show up on dbus and raise a signal
     let playerName = lcFirst(name);
     let newValue = 'foo';
     player[playerName] = newValue;
-    await ping();
+    await ping(bus);
     changed = {};
     changed[name] = new Variant('s', newValue);
     expect(cb).toHaveBeenCalledWith(ROOT_IFACE, changed, []);
@@ -99,9 +100,10 @@ test('setting properties on the player should show up on dbus and raise a signal
   expect(gotten).toEqual(new Variant('b', player.fullscreen));
   let newValue = !player.fullscreen;
   player.fullscreen = newValue;
-  await ping();
+  await ping(bus);
   changed = {
     Fullscreen: new Variant('b', newValue)
   };
   expect(cb).toHaveBeenLastCalledWith(ROOT_IFACE, changed, []);
+  await ping(bus);
 });