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

Error in TwitchChat Provider #228

Closed
degenone opened this issue Oct 8, 2023 · 1 comment · Fixed by #229
Closed

Error in TwitchChat Provider #228

degenone opened this issue Oct 8, 2023 · 1 comment · Fixed by #229

Comments

@degenone
Copy link
Contributor

degenone commented Oct 8, 2023

Before adding my Twitch credentials I got this error:

fail: Microsoft.Extensions.Hosting.Internal.Host[11]
      Hosting failed to start
      System.Collections.Generic.KeyNotFoundException: The given key 'ClientId' was not present in the dictionary.
         at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
         at TagzApp.Providers.TwitchChat.StartTwitchChat.MapConfigurationValues(ProviderConfiguration providerConfiguration) in C:\Users\Tero\git\TagzApp\src\TagzApp.Providers.TwitchChat\StartTwitchChat.cs:line 39
    ...

I think the null check in StartTwitchChat.MapConfigurationValues is never null and causes the KeyNotFoundException.

This can be fixed by changing MapConfigurationValues to have:

_TwitchChatConfiguration = new TwitchChatConfiguration
{
    ClientId = config.TryGetValue("ClientId", out string? id) ? id : string.Empty,
    ClientSecret = config.TryGetValue("ClientSecret", out string? secret) ? secret : string.Empty,
    ChatBotName = config.TryGetValue("ChatBotName", out string? botName) ? botName : string.Empty,
    OAuthToken = config.TryGetValue("OAuthToken", out string? token) ? token : string.Empty,
    ChannelName = config.TryGetValue("ChannelName", out string? channelName) ? channelName : string.Empty
};

Instead of this change, shoud we have it fail more gracefully in a try {} catch {}? Then let the user know those values are missing.

@csharpfritz
Copy link
Contributor

Ooh.. good catch. I'll put together a fix.

This feels like something that should surface in the configuration provider UI as well

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 a pull request may close this issue.

2 participants