Last.FM doesn't work with proxy settings

[rattboi]

OS: Windows 7

Issue Descriptions: Last.fm is blocked, so I have a socks proxy I use. I only want the traffic for GPMDP going through the proxy. I've enabled my proxy by adding '--proxy-server="socks5://localhost:8089"' to the gpmdp shortcut. I've tested that the music is using the proxy by stopping the proxy server, and seeing that music streaming no longer works, then starting the proxy server and streaming starts working again.

Steps to Reproduce:

1. Enable proxy (I know this isn't trivial for everyone) via --proxy-server="socks5://"
2. Go to "Desktop Settings -> Last.FM"
3. Click "Log in to Last.fm"
4. It shows "Authorization in progress" very briefly, then flips back to "Not Authorized"
5. It doesn't scrobble (obviously)

[rattboi]

Perhaps it's related to this? https://github.com/electron/electron/issues/183

[MarshallOfSound]

It is exactly that problem @rattboi and unfortunately that is quite tricky to fix. It would require the app using handling proxy settings internally which is quite complex

[jostrander]

Can we use something like https://www.npmjs.com/package/global-tunnel along with reading the command line settings to set the proxy for the node side of the web stack?

[MarshallOfSound]

I'm removing this from the v4 milestone and will implement using the new net module that is coming to Electron --> https://github.com/electron/electron/pull/7577

[rattboi]

FYI, looks like that PR got merged into Electron 15 days ago

[MarshallOfSound]

@rattboi I am aware :P

[asermax]

Any news on this? Is there any workaround to generate the credentials that we could use to have it working? I don't mind meddling with some code if necesary.

[MarshallOfSound]

@asermax It's not just the credentials that are the issue, the actual REST calls to trigger last.fm actions will face the same proxy issue. This isn't on my immediate todo list but if someone wants to pick it up that would be great 😄

[asermax]

Ooo, it makes sense :thinking: I would like to help but not very sure where to start. Giving a quick look at the related code I see that a third party module is being used to communicate with lastfm, do we need to replace it or configure it somehow? If you don't mind, would you give me a quick rundown on what your take on how to fix it?

In any case, I'll give it a more thorough look later today. Thanks for the promptly response 😛

[MarshallOfSound]

@asermax My initial reaction was "let's fork lastfm and change their usage of the 'http' module to be 'electron.net'". It's basically a drop in replacement.

Then I saw this. https://github.com/jammus/lastfm-node/blob/master/lib/lastfm/lastfm-request.js#L3

If we set global.GENTLY_HIJACK and use it to proxy the lastfm modules require('http') call to require('electron').net it might "Just Work"(tm). It's a bit of a hack but less work than maintaining a fork of lastfm.

[asermax]

Alright, after delaying this for a while I eventually got something working. Sadly I could not setup the dev environment on the windows machine I actually want to test the fix on (my work machine, where I'm having the proxy issue), so I made the change and tested it on my home comp in which I use linux. Aparently the monkey-patching works, but for further testing I'll (if you don't mind) open a PR as to get the windows executable built and actually test it at my work machine. I'll get back at you once I find out if it works or not 😛

[Gnarfoz]

@asermax did a Windows build of this ever happen? My previous player was a Chrome extension and thus had no problem following the system's proxy settings, but now I'm faced with this.

[Gnarfoz]

Actually, it did, AppVeyor did it: https://ci.appveyor.com/project/MarshallOfSound/google-play-music-desktop-player-unofficial/build/1.0.2007

[patryk-kawalarz]

Now last.fm doesn't work also without proxy. It doesn't work at all....

[rattboi]

I've finally gotten annoyed enough with this to look into it again. I have attempted the GENTLY_HIJACK fix, and after having some initial issues, I think I can say this works!

One thing to keep in mind is that normal build setup for this doesn't expose that you need to supply your own last.fm api/secrets. When I did this by overriding the GPMDP_LASTFM_KEY, etc, and then re-authorizing via the gui, it started scrobbling!

@MarshallOfSound I think you must be keeping the key/secret in your CI infra, and so it's left kinda on the floor for developers.

Keep in mind that this is on OSX, so your results may vary. I am attempting to package for myself to see if it continues to work. Will report back.

[rattboi]

I had some packaging difficulties because of weirdness with electron-packager and npm 5+. After using npm 3, I was able to package, and I'm running a working, scrobbling, proxied GPMDP. I think we can confirm this works, and we should get an official build so it starts using the official last.fm key/secrets.

@MarshallOfSound is there a way to get a signed beta build based on this, for testing?

[MarshallOfSound]

@rattboi On OSX all you need to do is make a PR and have it merged, once it's merged circleci will built a signed version of the app for you 👍

[rattboi]

@MarshallOfSound I'm just using #2241 changes and then I built using my last.fm key/secret, and that's working. I don't want to make a new PR, since this is actually @asermax's work. If there's no way around it, I'll make a new PR and try to attribute it to him. I think you can just re-open #2241 and merge that though...

[asermax]

Don't worry about attributing it to me, go for whatever is easier :) I'm glad to see it helped tho!

[rattboi]

@MarshallOfSound would you prefer I open a new PR, or can we merge @asermax's original PR (#2241)?