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

fix: ignored TWT_TOKEN env variable #592

Merged
merged 1 commit into from
May 12, 2024
Merged

Conversation

Yokann
Copy link
Contributor

@Yokann Yokann commented May 5, 2024

Hi !

Discovered this tool yesterday. I wondered why I can't pass TWT_TOKEN env variable to the CLI. It seems the variable is ignored.

I made this fix. It should deserve a test, but I don't know Rust so much.

Copy link

codecov bot commented May 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 19.26%. Comparing base (882e816) to head (d368c95).
Report is 1 commits behind head on main.

Files Patch % Lines
src/handlers/config.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #592   +/-   ##
=======================================
  Coverage   19.26%   19.26%           
=======================================
  Files          41       41           
  Lines        5086     5086           
=======================================
  Hits          980      980           
  Misses       4106     4106           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Xithrius
Copy link
Owner

Xithrius commented May 5, 2024

Hi! Thanks for the PR. Don't worry about the failing CI on formatting, I'll be testing this just on functionality.

@Xithrius Xithrius self-requested a review May 6, 2024 21:01
@Xithrius Xithrius added type: bug Something isn't working area: backend Internal enhancements and removed area: backend Internal enhancements labels May 6, 2024
Copy link
Owner

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

I'm not really seeing what your diff does to fix the bug that you were experiencing. You changed the control flow a little bit, but the logic for checking and setting the environment variable is done before the changes you made.

The section that you did change deals with the error handling, but not any actual writing to the token environment variable that is used to log into the IRC at config.twitch.token.

Perhaps it's something to do with exporting TWT_TOKEN on your system. What shell are you using, and how are you setting the environment variable? I am unable to reproduce via export "<token>" && twt.

@Yokann
Copy link
Contributor Author

Yokann commented May 7, 2024

Hi,
If I tried to pass the variable as environment variable, I get this error. So I looked into the checking part.

Twitch config section is missing one or more of the following: username, channel, token.

I don't think the error is relative to a shell, as I got the same result from bash or zsh on Arch.

Here is the part of my script I am trying to run. The $ACCESSTOKEN is valid, tested directly in the twt config file.

streamlink -Q "--twitch-api-header=Authorization=Bearer $ACCESSTOKEN" https://www.twitch.tv/$selected_channel &
TWT_TOKEN="oauth:$ACCESSTOKEN" twt -c $selected_channel

Let me do some recheck. It's weird.

@Yokann
Copy link
Contributor Author

Yokann commented May 7, 2024

I also tried export TWT_TOKEN=*****. It doesn't change anything.

@Xithrius
Copy link
Owner

Xithrius commented May 10, 2024

What version of twitch-tui are you on? I tried the same command as yours with my token, seemed to work fine.

@Yokann
Copy link
Contributor Author

Yokann commented May 10, 2024

I'm using twitch-tui 2.6.7 and rust 1.78.0. I did a fresh install on a different machine (Macbook) and I've got the same issue.

Fun story, and that's why I thought my PR fix the issue, when I run from the source, it works totally fine

TWT_TOKEN=*** cargo run

but if I run it with the binary, the environment variable is ignored. :mindblowing:

TWT_TOKEN=*** ./target/debug/twt

@Xithrius
Copy link
Owner

Xithrius commented May 10, 2024

Do the changes in this PR fix the issue you're experiencing?

@Yokann
Copy link
Contributor Author

Yokann commented May 12, 2024

I updated the PR and changed the way the code get the variable, and everything seems fine now.
Maybe an issue with the option_env macro.

@Xithrius
Copy link
Owner

Ah, yeah that might just fix it. I'll test it very soon. I have a feeling it's to do with option_env's way of checking for the variable:

Optionally inspects an environment variable at compile time.

Copy link
Owner

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

I've tried a bunch of different ways to set the environment variable, I haven't had any problems with the current state of this PR.

I'll get out a version such that the hotfix is available.

Thank you!

@Xithrius Xithrius merged commit a1bf3ff into Xithrius:main May 12, 2024
6 of 8 checks passed
@Xithrius
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants