search

tywil04 / slavartdl

Simple tool written in Go(lang) to download music using the SlavArt Divolt server
GNU General Public License v3.0
18 stars 1 forks source link

Session token not being saved to config #11

Closed bacon-cheeseburger closed 1 year ago

bacon-cheeseburger commented 1 year ago

Hi, I start the app pointed to a new config json and the default config is generated. I then add my login credentials to it and re-run the app. The session token is fetched an download message sent but it seems the session token isn't saved to the config. The download failed due to the link quota exceeded problem that happens sometimes. Not sure if the token is saved after a successful download, or if it's saved at all for that matter. I assumed it would be saved automatically after being successfully fetched so not sure if this is an actual bug or bad assumption on my part.

On a side note, it might be helpful to be able to set the login/password from command line rather than hand edit the config. That would eliminate people accidentally excluding things like necessary curly brackets and commas like I did. :\

tywil04 commented 1 year ago

The quota exceeded problem is out of my control, there have been some issues with the bot on divolt for the past few days. I guess one thing I should do is identify the page and return an appropriate error to the user instead of just timing out.

Regarding the login credentials, its designed so every time you run the command it creates a new session token and uses that. This is wasteful but it seemed odd to remove the users login credentials from the config and add a session token - I don't feel as if there would be an effective way to communicate this to the user.

I do agree that like other options in the config there should be commands to add/remove credentials.

tywil04 commented 1 year ago

Maybe instead of allowing credentials in the config I make it so you can login using your credentials using a dedicated command and the resulting session token is added to either the default config or the config provided when running the command.

Do you think this could be a better approach than what is implemented currently?

bacon-cheeseburger commented 1 year ago

Actually I think the current behavior is best/simplest for users. I just thought the behavior was something else (tokens saved for possible use with future sessions before fetching a new one). Now that the app can fetch tokens itself, and unless I'm missing something, I don't really see a need to save/track tokens for re-use over multiple sessions/app runs.

I think adding commands to set credentials (no need to remove them, can just re-set them) from the config, and returning more helpful errors (if you know why the error occurred) are the only suggestions I'd make now. To keep the user-facing credential stuff simple, I guess the logic could be something like:

  1. Attempt to get credentials from config. If missing, inform user they need to add credentials with the appropriate commands and app exits. This loop continues until credentials have been added.

  2. Attempt to get session token using loaded credentials. If failed, inform user of (possible) problems (login failure/invalid credentials (typo?), can't connect to server, etc) and app exits.

  3. If you made it this far, you know you have working credentials and a valid token.

tywil04 commented 1 year ago

Credentials can now be managed with the command line: https://github.com/tywil04/slavartdl/commit/55f9d20f4889382ae1e8db459fdf72dada0963a3, I will be working on improving the errors in the near future.