plugins.twitch: better access token error handling #5011
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TwitchAPI
class:CLIENT_ID
as a class attribute (for test references)call()
to pass custom headers (currently unused)json.dumps()
PlaybackAccessToken
error response and log error messagesOptions.clear()
for resetting plugin options state after testsResolves #5009
This adds error logging to the access token processing and also adds tests for the access token responses and how the
--twitch-api-header
and--twitch-access-token-param
influence the access token request.Small side-note in regards to the added
Options.clear()
method:I've already mentioned somewhere in the
Plugin.bind()
removal in 5.0.0 (#4744 / #4768) thatPlugin.options
will also need to get fixed at some point. Similar to the previously bound class attributes,Plugin.options
is a class attribute that gets shared accross all plugin classes unless it gets overridden bystreamlink_cli.main.setup_plugin_args()
. This means that when streamlink_cli is not involved in plugin construction, e.g. in tests or when using the Streamlink session API directly, setting any kind of plugin option always requires clearing the options, which is bad. I'm pretty sure that there are a couple of tests which are polluting the shared plugin options dict. I'll check once I get the time.The reason why I didn't touch
Plugin.options
at the time is that removingPlugin.bind()
was already a lot of work. In hindsight, this should've been done in one go, because changing it will probably be a breaking API change, because of how the plugins will need to get initialized. Currently, the options are set by the session instance before plugins are initialized, and it expects the options dict to be available on the plugin class. Options should however only be set on plugin instances, so there's no shared options dict, neither between plugin classes, nor between different instances of the same plugin.I will need to have a closer look at this and figure out a better solution before making any further comments. This PR should be fine though.