svrooij / node-sonos-ts

:speaker: Sonos control library, use this library in your own appliction.
https://sonos-ts.svrooij.io/
MIT License
81 stars 18 forks source link

BUG: Error thrown in MetadataHelper.ParseDIDLTrack #192

Closed hklages closed 5 months ago

hklages commented 5 months ago

What happened

await tsCoordinator.AVTransportService.GetMediaInfo() throws the error (it is packaged by my module):

"TypeError: uri.replace is not a function\n    
at MetadataHelper.ParseDIDLTrack (C:\\Users\\hekla\\.node-red\\node_modules\\node-red-contrib-sonos-plus\\node_modules\\@svrooij\\sonos\\lib\\helpers\\metadata-helper.js:56:29)\n    
at AVTransportService.parseValue (C:\\Users\\hekla\\.node-red\\node_modules\\node-red-contrib-sonos-plus\\node_modules\\@svrooij\\sonos\\lib\\services\\base-service.js:253:46)\n    
at C:\\Users\\hekla\\.node-red\\node_modules\\node-red-contrib-sonos-plus\\node_modules\\@svrooij\\sonos\\lib\\services\\base-service.js:246:30\n    
at Array.forEach (<anonymous>)\n    
at AVTransportService.parseEmbeddedXml (C:\\Users\\hekla\\.node-red\\node_modules\\node-red-contrib-sonos-plus\\node_modules\\@svrooij\\sonos\\lib\\services\\base-service.js:245:14)\n    
at AVTransportService.handleRequestAndParseResponse (C:\\Users\\hekla\\.node-red\\node_modules\\node-red-contrib-sonos-plus\\node_modules\\@svrooij\\sonos\\lib\\services\\base-service.js:205:21)\n    
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    
at async AVTransportService.GetMediaInfo (C:\\Users\\hekla\\.node-red\\node_modules\\node-red-contrib-sonos-plus\\node_modules\\@svrooij\\sonos\\lib\\services\\av-transport.service.js:127:60)\n    
at async Object.groupGetTrackPlus [as group.get.trackplus] (C:\\Users\\hekla\\.node-red\\node_modules\\node-red-contrib-sonos-plus\\src\\sonos-universal.js:819:23)"

when playing the http stream: "https://kexp.streamguys1.com/kexp160.aac", initiated via "await tsCoordinator.AVTransportService.SetAVTransportURI({ 'InstanceID': 0, 'CurrentURI': validatedUri, 'CurrentURIMetaData': metadata })"

where validatedUri: "https://kexp.streamguys1.com/kexp160.aac" and CurrentUriMetadata was created with "await MetaDataHelper.TrackToMetaData(track, false)"

where track: { Title: "", AlbumArtUri: "",

end also html encoded.

Can you reproduce the error? BTW: When starting the stream via Sonosplayer, the issue does not show up.

What did you expect to happen

No exception :-) and returning the media info.

How to reproduce it (minimal and precise)

see above

Debug logging

see above

Environment

svrooij commented 5 months ago

@hklages this is not really a you can reproduce it by executing this, then this...

I'm not sure if I can help out with this.

The URL you're sending is not supported by the Metadata helper. https://sonos-ts.svrooij.io/sonos-device/methods.html#metadata

hklages commented 5 months ago

Thanks. If that is not supported we can close this issue.

On Tue, Mar 26, 2024, 21:33 Stephan van Rooij @.***> wrote:

@hklages https://github.com/hklages this is not really a you can reproduce it by executing this, then this...

I'm not sure if I can help out with this.

The URL you're sending is not supported by the Metadata helper. https://sonos-ts.svrooij.io/sonos-device/methods.html#metadata

— Reply to this email directly, view it on GitHub https://github.com/svrooij/node-sonos-ts/issues/192#issuecomment-2021416234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEDZCH6F3OMCFGGXJODTDLTY2HEQFAVCNFSM6AAAAABFI2QHHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRRGQYTMMRTGQ . You are receiving this because you were mentioned.Message ID: @.***>

svrooij commented 5 months ago

I was just saying that generating metadata on that track was not supported. I'm wondering if it changes if you play the song by the Sonos app.

Let's say give me a script version which I can use to debug here. Take any of the samples from the examples directory modify it a bit so that my player starts playing your song and then trigger the error.

Then I'll probably be able to fix this in minutes.

hklages commented 5 months ago

Ok - I will do.

I guess there is something wrong with the "didlItem['upnp:albumArtURI'])". Maybe it is not array of string or string in that specific stream and therefore will not allow a ".replace()"

svrooij commented 5 months ago

I guess that the album URI might be null, and null does not have a replace.

hklages commented 5 months ago
if (didlItem['upnp:albumArtURI']) {
     const uri = Array.isArray(didlItem['upnp:albumArtURI']) ? didlItem['upnp:albumArtURI'][0] : didlItem['upnp:albumArtURI'];
     // Github user @hklages discovered that the album uri sometimes doesn't work because of encoding:
     // See https://github.com/svrooij/node-sonos-ts/issues/93 if you found and album art uri that doesn't work.
     const art = uri.replace(/&amp;/gi, '&'); // .replace(/%25/g, '%').replace(/%3a/gi, ':');
     track.AlbumArtUri = art.startsWith('http') ? art : `http://${host}:${port}${art}`;
}

... and the leading if does not ensure that it is string or array of string (at least not in JS)

hklages commented 5 months ago

Issue

Calling AVTransportService.GetMediaInfo() throws an error „uri.replace is not a function“ at line 56 (.js) in ParseDIDLTrack. The reason for that is

In beta-5 an empty tag is not a problem. In beta-5 and beta-7 I can avoid the error message by adding a single blank <upnp:albumArtURI> </upnp:albumArtURI> in the call to set the stream. Thats what I did to fix the error.

Root cause

The new parser seems to return an empty object „{}“ in case of an empty XML tag. Thats a breaking change compared to the previous parser.

Suggestion:

The if clause should check on (string or array with first element is string) instead of just if (didlItem['upnp:albumArtURI']) {..} Maybe you should also check the part for title and others.

Here is a small program - no errror handling. ts_sonos_tests.zip

Happy Eastern - it is not urgent, as I fixed it in my package already and the original issue is closed.

svrooij commented 5 months ago

You could also have done it without the self xml generation. The code below will generate the metadata, this is arranged by this code

  const args = {
    'InstanceID': 0,
    'CurrentURI': 'x-rincon-mp3radio://https://kexp.streamguys1.com/kexp160.aac',
    'CurrentURIMetaData': {
      CdUdn: 'RINCON_AssociatedZPUDN',
      ItemId: '-1'
    }
  }
  await kidsPlayer.AVTransportService.SetAVTransportURI(args)

And if you must generate the xml yourself, XmlHelper.EncodeXml(...)

I will however fix the issue that each used value should not be empty before doing a replace

hklages commented 5 months ago

Regarding Metadata -> good hint, simplifies the code, will implement it with the next release Regarding fix -> Thanks.

svrooij commented 5 months ago

@hklages I'll do you one better 😉 I'll start supporting the url you're sending in the metadata helper.

  await sonos.SetAVTransportURI('x-rincon-mp3radio://https://kexp.streamguys1.com/kexp160.aac');

  //await sonos.SwitchToQueue();
  await sonos.Play();
  console.log('SetAVTransportURI done');
  console.log(await sonos.AVTransportService.GetMediaInfo());
  console.log(await sonos.AVTransportService.GetPositionInfo());

returns the following:

SetAVTransportURI done
{
  NrTracks: 1,
  MediaDuration: 'NOT_IMPLEMENTED',
  CurrentURI: 'x-rincon-mp3radio://https://kexp.streamguys1.com/kexp160.aac',
  CurrentURIMetaData: {
    Album: undefined,
    Artist: undefined,
    AlbumArtUri: undefined,
    Title: undefined,
    UpnpClass: undefined,
    Duration: undefined,
    ItemId: -1,
    ParentId: undefined,
    TrackUri: undefined,
    ProtocolInfo: undefined
  },
  NextURI: {},
  NextURIMetaData: {},
  PlayMedium: 'NETWORK',
  RecordMedium: 'NOT_IMPLEMENTED',
  WriteStatus: 'NOT_IMPLEMENTED'
}
{
  Track: 1,
  TrackDuration: '0:00:00',
  TrackMetaData: {
    Album: undefined,
    Artist: 'Hurray for the Riff Raff',
    AlbumArtUri: undefined,
    Title: 'Hurray for the Riff Raff',
    UpnpClass: 'object.item',
    Duration: undefined,
    ItemId: -1,
    ParentId: -1,
    TrackUri: 'aac://https://kexp.streamguys1.com/kexp160.aac',
    ProtocolInfo: 'aac:*:application/octet-stream:*'
  },
  TrackURI: 'aac://https://kexp.streamguys1.com/kexp160.aac',
  RelTime: '0:00:05',
  AbsTime: 'NOT_IMPLEMENTED',
  RelCount: 2147483647,
  AbsCount: 2147483647
}
github-actions[bot] commented 5 months ago

:tada: This issue has been resolved in version 2.6.0-beta.8 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

hklages commented 5 months ago

oh - great. Then I dont have to change anything. I will test it in the next days.