Alxandr / SpotiFire

A project to make a SpotifyClient in C#
http://nudoc.azurewebsites.net/SpotiFire
40 stars 19 forks source link

awaiting Link AsTrack never completes. #63

Closed woellij closed 8 years ago

woellij commented 8 years ago

Hi there,

firstofall thanks for the great library. Without it i would be totally lost ;)

In my Project i'm heavily relying on the Spotify Web Api - for everything except the Playback of course - for which i use SpotiFire.

Though transitioning the Web Api track responses to SpotiFire tracks caused me some headaches.

I'm using the Session's GetLink / ParseLink Methods to create a Link object from a Spotify URI (e.g. "spotify:track:2cOulAp3KOxbDSgbJGL6EO")

Then i'm using the Link's AsTrack Method to convert it to a track and i'm awaiting that ... but it never completes (sometimes it helps waiting for a few hundred milliseconds between the call to GetLink and AsTrack - but as soon as there is no Debugger attached / Deployed in Release Mode it stops working)

I've uploaded a sample project to OneDrive.

http://1drv.ms/1JZ03GI

I Hope you can help me out .. maybe i'm doing something completely wrong but i could reproduce the issue in the Sample Console App aswell as in the WPF Application i'm working on.

Many thanks

Alxandr commented 8 years ago

I won't say for certain, but the problem might be as simple as you running while (true) {} on the main thread (never do that). Or there might be some glaring error in SpotiFire. It's not been developed for a good while, and I'm waiting for Spotify to drop their new library (which they have stated is coming) before rewriting the entire thing.

brianavid commented 8 years ago

Hi,

Your split between web API and SpotiFire is the same as in my project ([https://github.com/brianavid/Avid4G.Net]). And I have no problem awaiting the result of Session.GetLink().AsTrack().

But I am concerned as the addition of Session.GetLink() was my recent change (in order to allow interop with the web API).

And I am confused, as I can't find any method Session.ParseLink() in LibSpotify!!

The relevant bits if my project code are:

async Task<Boolean> PlayTrackAsync(
    Track track,
    bool append = false)
{
    if ((await track).IsAvailable)
    {
        try
        {
            SpotifySession.EnqueueTrack(track, append);

        }
        catch (Exception ex)
        {
            logger.Warn(ex);
            throw new HttpResponseException(HttpStatusCode.InternalServerError);
        }
    }
    return true;
}

/// <summary>
/// Play the identified track, either immediately or after the currently queued tracks
/// </summary>
/// <param name="id"></param>
/// <param name="append"></param>
/// <returns></returns>
[HttpGet]
[LoggingExceptionFilter]
public Boolean PlayTrack(
    string id,
    bool append = false)
{
    try
    {
        Track track = SpotifySession.GetTrack(id);
        if (track == null)
        {
            return false;
        }
        return PlayTrackAsync(track, append).Result;
    }
    catch (Exception ex)
    {
        logger.Warn(ex);
        return false;
    }
}

------------------

internal static Track GetTrack(
    string id)
{
    if (string.IsNullOrEmpty(id))
    {
        return null;
    }

    Link link = Session.GetLink(id);

    return link == null ? null : link.AsTrack();
}
woellij commented 8 years ago

Hi,

thanks for your feedback. It's nice to see you're still active on the project despite not actively developing it anymore.

I've looked into it myself this morning and found the problem (an error "SpotifyLibExtensions" using the timer to create the TaskAwaiter)

I'll submit a pull request later today.

(The ParseLink Method is an Extension Method in SpotiFire.LinkExtensions. Extension Methods only get really good and discoverable with resharper ;) )

NeoLegends commented 8 years ago

Oh, thats great. I've had problems with that myself, and I indeed did a while (!track.IsLoaded) await Task.Delay(10); to work around the problem.

brianavid commented 8 years ago

I've obviously always been lucky and don't get the hang in my system.

And I never spotted ParseLink(), as I don't have ReSharper. So I never needed to add Session.GetLink() after all - the capability already was available :-(

On Thu, 4 Feb 2016 at 09:10 Moritz Gunz notifications@github.com wrote:

Oh, thats great. I've had problems with that myself, and I indeed did a while (!track.IsLoaded) await Task.Delay(10); to work around the problem.

— Reply to this email directly or view it on GitHub https://github.com/Alxandr/SpotiFire/issues/63#issuecomment-179722352.

woellij commented 8 years ago

@brianavid I've checked out your project last night during my research on the bug :) You've done some very impressive work :) :+1:

I think the reason why you haven't run into that 'await bug' yet is because you create the tracks some time before you start playing them.

In my scenario (and probaly in @NeoLegends ' aswell) i'm creating the link and awaiting the track just right before i want to play it (it hasn't had any time yet to be loaded in the background - and the "await facade" implementation caused the never ending wait in that case.

I've added an extra method to the Test Client Console App that demonstrates that awaiting the Track right after creating it from a Link now works.

The pull request has also been issued and i'm glad i could help another poor soul along the way ;)

NeoLegends commented 8 years ago

Your pull request looks pretty good, it lacks some locking though. :)

And I'm beginning to think about dropping the foreach entirely and replacing it with a much simpler for (int i = list.Count - 1; i >= 0; i--) { list.RemoveAt(i); }. Since this way allows iterating and removing at the same time, we can also avoid taking a copy of the list at the beginning.

woellij commented 8 years ago

Yeah that makes sense. i implemented it that way and added some more "tests" in the TestClient Program (at the time i was just looking for a quick fix ;) )

NeoLegends commented 8 years ago

Packages are pushed, you all can update your dependencies now.