Returning more detailed error message: contentDirectoryService().GetResult

hklages commented 4 years ago

Is it possible to get a more detailed error message from GetResult? I would like to act on the root cause of the error. If the player is not reachable -> inform the user to verify ip. If the queue is empty let the user fill the queue.

Expected Behavior

GetResult: Return an error I can act on - not only 'false'.

Apart from getQueue, the methodsgetMusicLibrary('sonos_playlists', * ), getMusicLibrary('playlists',*), getFavorites() use GetResults.

Current Behavior

Several commands (e. g. getQueue) uses contentDirectoryService().GetResult. GetResults returns 'false' if anything goes wrong for example the queue is empty or the player is not reachable.

Possible Solution

return 'true' if the queue is empty and in all other cases 'false' .. or ..
if the queue is empty return the full object (empty array items, total=0, ...)

Versions (and Environment)

Node version: 10.15.2 node-sonos version: 1.12.5 NodeRED: 1.0.2

bencevans commented 4 years ago

Agreed this would be more useful. Another option might be to return instances of certain errors which can then be checked against. PR welcome! :smiley:

hklages commented 4 years ago

Hi. It took a bit to find the root cause. But please check I am not 100% sure.

It seem to me that the root cause is in

lib/services/Service.js
function Service.prototype._request

Error handling is: .catch(reject)

I suggest:

.catch((error) => {
        // In case of an SOAP error error.reponse helds the details.
        // That goes usually together with status code 500 - triggering catch
        // Experience: When using reject(error) the error.reponse get lost.
        // Thats why error.response is checked and handled here
        let myError;
        if (error.response) { // Indicator for SOAP Error
          if (error.message.startsWith('Request failed with status code 500')) {
            const errorCode = error.response.data;
            myError = new Error('upnp: statusCode 500 & upnpErrorCode ' + error.response.data);
            reject(myError);
          } else {
            myError = new Error('upnp: ' + error.message + '///' + error.response.data);
            reject(myError);
          }
        } else {
          reject(error);
        }
      });

I handled it in a similar way when sending SOAP parameters direct with Axios and could receive error codes like: 401 - invalid action or 712: unsupported playmode.

Looks similar to Performing SOAP Requests

svrooij commented 4 years ago

I do agree with your suggestion on checking the error responses, but there is a catch.

The error code is available, but sonos doesn't send the optional (but recommended errorDescription). See specs page 86 for more details.

So when talking to sonos you can only get the codes like 402 or 712. and not invalid playmode.

And you're also right, on error is should just throw an error, not sure why I haven't implemented that everywhere. But when no results I would suggest an empty array, what do you think @hklages

hklages commented 4 years ago

Hi, I know. Sonos does not provide text error messages. But I found a way to translate the code :-)

SONOS_ERROR_CODES: { // from https://raw.githubusercontent.com/tkugelberg/SymconSonos/master/Sonos/sonosAccess.php '/MediaRenderer/AVTransport/Control': [ { code: 701, message: 'ERROR_AV_UPNP_AVT_INVALID_TRANSITION' }, { code: 702, message: 'ERROR_AV_UPNP_AVT_NO_CONTENTS' }, { code: 703, message: 'ERROR_AV_UPNP_AVT_READ_ERROR' }, { code: 704, message: 'ERROR_AV_UPNP_AVT_UNSUPPORTED_PLAY_FORMAT' }, { code: 705, message: 'ERROR_AV_UPNP_AVT_TRANSPORT_LOCKED' }, { code: 706, message: 'ERROR_AV_UPNP_AVT_WRITE_ERROR' }, { code: 707, message: 'ERROR_AV_UPNP_AVT_PROTECTED_MEDIA' }, { code: 708, message: 'ERROR_AV_UPNP_AVT_UNSUPPORTED_REC_FORMAT' }, { code: 709, message: 'ERROR_AV_UPNP_AVT_FULL_MEDIA' }, { code: 710, message: 'ERROR_AV_UPNP_AVT_UNSUPPORTED_SEEK_MODE' }, { code: 711, message: 'ERROR_AV_UPNP_AVT_ILLEGAL_SEEK_TARGET' }, { code: 712, message: 'ERROR_AV_UPNP_AVT_UNSUPPORTED_PLAY_MODE' }, { code: 713, message: 'ERROR_AV_UPNP_AVT_UNSUPPORTED_REC_QUALITY' }, { code: 714, message: 'ERROR_AV_UPNP_AVT_ILLEGAL_MIME' }, { code: 715, message: 'ERROR_AV_UPNP_AVT_CONTENT_BUSY' }, { code: 716, message: 'ERROR_AV_UPNP_AVT_RESOURCE_NOT_FOUND' }, { code: 717, message: 'ERROR_AV_UPNP_AVT_UNSUPPORTED_PLAY_SPEED' }, { code: 718, message: 'ERROR_AV_UPNP_AVT_INVALID_INSTANCE_ID' }

bencevans commented 4 years ago

:tada: This issue has been resolved in version 1.13.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

bencevans / node-sonos