Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First implementation of automatic recognition of redirect url #98

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

jfaltis
Copy link
Contributor

@jfaltis jfaltis commented Oct 22, 2019

This is my first approach of implementing #96. The local webserver now starts just before the token request and listens for incoming GET requests. When a get request is received it tries to "parse" the URL. If the start of the webserver fails or the GET request cant be parsed it returns to manual authentication (old way).

The problem I see with the current state of my implementation:

  • If the redirect url is not aimed at port 8888 but at a different port (since this can be configured in the Spotify Dashboard) the program would wait for ever because it won't receive a GET request. I guess I have to implement some kind of timeout after which the program will return to manual authentication.

What do you think?

Copy link
Owner

@Rigellute Rigellute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @jfaltis!

Not sure how concerned we should be if the redirect URI was entered incorrectly to Spotify, however, I like your suggestion of adding a timeout.

This could probably be implemented later though.

I'm going to merge this into another branch so I can test before we merge to master.

Really great work!

@Rigellute Rigellute changed the base branch from master to auto-redirect-code October 23, 2019 08:07
@Rigellute Rigellute merged commit fb544aa into Rigellute:auto-redirect-code Oct 23, 2019
@Rigellute Rigellute mentioned this pull request Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants