discord-net / Discord.Net

An unofficial .Net wrapper for the Discord API (https://discord.com/)
https://discordnet.dev
MIT License
3.34k stars 737 forks source link

Requiring cultures does not seem necessary #2704

Open LucHeart opened 1 year ago

LucHeart commented 1 year ago

Discord.NET version

3.10.0

.NET version

7

Docker base image

mcr.microsoft.com/dotnet/runtime:7.0-alpine

Dockerfile to rep

FROM mcr.microsoft.com/dotnet/sdk:7.0-alpine AS build

WORKDIR /src
COPY . .
WORKDIR "/src/DiscordBot"

RUN dotnet publish "DiscordBot.csproj" -c Release -o /app/publish

FROM mcr.microsoft.com/dotnet/runtime:7.0-alpine
WORKDIR /app
COPY --from=build /app/publish .

ENTRYPOINT ["dotnet", "DiscordBot.dll"]

csproj to rep

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>net7.0</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
        <AssemblyVersion>1.0.0.0</AssemblyVersion>
        <FileVersion>1.0.0.0</FileVersion>
        <AssemblyName>DiscordBot</AssemblyName>
        <RootNamespace>DiscordBot</RootNamespace>
    </PropertyGroup>

    <ItemGroup>
      <PackageReference Include="Discord.Addons.Hosting" Version="5.2.0" />
      <PackageReference Include="Discord.Net" Version="3.10.0" />
    </ItemGroup>

</Project>

Background information

We were trying to just run a empty bot on a alpine dotnet7 runtime base image in docker. However we were immediately greeted with the error:

18:55:47 Gateway     Connected
18:55:47 Gateway     Error handling Dispatch (GUILD_AVAILABLE):

System.Globalization.CultureNotFoundException: Only the invariant culture is supported in globalization-invariant mode. See https://aka.ms/GlobalizationInvariantMode for more information. (Parameter 'name')

en-US is an invalid culture identifier.

   at System.Globalization.CultureInfo..ctor(String name, Boolean useUserOverride)

   at Discord.WebSocket.SocketGuild.Update(ClientState state, Guild model)

   at Discord.WebSocket.SocketGuild.Update(ClientState state, ExtendedGuild model)

   at Discord.WebSocket.DiscordSocketClient.ProcessMessageAsync(GatewayOpCode opCode, Nullable`1 seq, String type, Object payload)

We then figured out that Discord.NET requires en-US as a culture. So the fix was simple: Add <InvariantGlobalization>false</InvariantGlobalization> to your csproj and add RUN apk add --no-cache icu-libs to your dockerfile. That then fixed the issue we had so far, however I was questioning why it would require cultures. I think it should support running with invariant culture aswell.

So after talking to someone on the discord server about it they said it has the following reason and told us to open a github issue about it.

eggsbox on Discord.NET's discord server:

It's for registerring the PreferredCulture for a guild based on the Locale
so with it running in InvariantMode; and alpine having no cultures, it couldn't load it and dnet isn't safely catching it
I'd suggest raising it as an issue on the github with your stack trace.

Feel free to request any more information / reach out.

WardenDrew commented 11 months ago

Ran into the same issue. I had found the csproj flag but was missing the dockerfile change so that was helpful to find @LucHeart

Pretty much the same exception stack here:

fail: WardenBot.Discord.BackgroundServices.DiscordBackgroundService[0]
      [Gateway] Error handling Dispatch (GUILD_AVAILABLE)
      System.Globalization.CultureNotFoundException: Only the invariant culture is supported in globalization-invariant mode. See https://aka.ms/GlobalizationInvariantMode for more information. (Parameter 'name')
      en-US is an invalid culture identifier.
         at System.Globalization.CultureInfo..ctor(String name, Boolean useUserOverride)
         at Discord.WebSocket.SocketGuild.Update(ClientState state, Guild model)
         at Discord.WebSocket.SocketGuild.Update(ClientState state, ExtendedGuild model)
         at Discord.WebSocket.DiscordSocketClient.ProcessMessageAsync(GatewayOpCode opCode, Nullable`1 seq, String type, Object payload)
delasource commented 8 months ago

+1 here

if you have an application with <InvariantGlobalization>true</InvariantGlobalization> you can not use this library. Should be fixed asap, since some of the "dotnet new" templates come with this mode enabled by default!

rpendleton commented 6 months ago

Another way to reproduce issues related to this is to use dotnet's official chiseled Docker images. Similar to Alpine docker images, these chiseled images also exclude ICU data.

When I tried using Discord.NET with mcr.microsoft.com/dotnet/runtime:8.0-jammy-chiseled, I ran into stack traces that match #2795. Switching to mcr.microsoft.com/dotnet/runtime:8.0-jammy-chiseled-extra fixed those exceptions.

GedasFX commented 2 months ago

Can verify it still. I moved to alpine and got this message. Is this going to be fixed in v4?

quinchs commented 2 months ago

from what I understand of this issue, we dont want CultureInfo? Would a wrapper type make sense for it that contains the identifier and a method/prop to get a CultureInfo instance, or should we just go primitive with strings.

V4 currently uses CultureInfo, but im open to change that.

LucHeart commented 2 months ago

Can verify it still. I moved to alpine and got this message. Is this going to be fixed in v4?

Verymuch still current, this is still an issue till this day. workaround I provided still works

LucHeart commented 2 months ago

from what I understand of this issue, we dont want CultureInfo? Would a wrapper type make sense for it that contains the identifier and a method/prop to get a CultureInfo instance, or should we just go primitive with strings.

V4 currently uses CultureInfo, but im open to change that.

Im not sure what is the best option here, as I probably dont see the full picture. But here are a few thoughts that come to mind

  1. Changing it to use strings everywhere could work, but I dont think that using CultureInfo is convenient and type and os backed, so this is my least preferred outcome personally.
  2. Somehow make it optional, not a hard requirement that crashes your application on runtime startup
  3. Not deal with this on a code base level, but more so providing a docs section about running this on a minimal os image, like alpine in a container, and provide an example on how to configure your system or container
GedasFX commented 2 months ago

from what I understand of this issue, we dont want CultureInfo? Would a wrapper type make sense for it that contains the identifier and a method/prop to get a CultureInfo instance, or should we just go primitive with strings.

V4 currently uses CultureInfo, but im open to change that.

I took a gander here: https://learn.microsoft.com/en-us/dotnet/core/docker/container-images#images-suitable-for-globalization

Basically if there is some built in need for globalization, we must use <InvariantGlobalization>false</InvariantGlobalization>. The reason why we get this error is simply because the OS does not have the globalization files. From doc, best we can do is runtime-deps:8.0-bookworm-slim?

But I genuinely do not see why this library does not just use invariant, does it have culture specific operations?

GedasFX commented 2 months ago

Note that <InvariantGlobalization> is false by default. I do not know if it adds additional linting checks.

GedasFX commented 2 months ago

https://github.com/dotnet/runtime/blob/main/docs/design/features/globalization-invariant-mode.md

Here's more reading. But yeah, in short, this does come with big limitations, but I do not see how culture info is needed within the library. Things like IgnoreCase stop working for characters outside of ASCII range.

LucHeart commented 2 months ago

Note that <InvariantGlobalization> is false by default. I do not know if it adds additional linting checks.

true, thats what I also expected, but the last time I tried without that, it somehow didnt work, so maybe there is a runtime config somewhere which sets a different default, not sure.

GedasFX commented 2 months ago

Note that <InvariantGlobalization> is false by default. I do not know if it adds additional linting checks.

true, thats what I also expected, but the last time I tried without that, it somehow didnt work, so maybe there is a runtime config somewhere which sets a different default, not sure.

I believe this should be set in the project itself (Discord.NET).

Misha-133 commented 2 months ago

does it have culture specific operations?

fyi, InteractionService uses culture info for loading & registering command localizations

GedasFX commented 2 months ago

Did a small scale test:

<InvariantGlobalization>true</InvariantGlobalization>
CultureInfo.InvariantCulture // Returns "Invariant Language (Invariant Country)"
CultureInfo.CurrentCulture // Returns "Invariant Language (Invariant Country)"
CultureInfo.GetCultureInfo("en-US") // Throws "System.Globalization.CultureNotFoundException: Only the invariant culture is supported in globalization-invariant mode. See https://aka.ms/GlobalizationInvariantMode for more information."
<InvariantGlobalization>false</InvariantGlobalization>
CultureInfo.InvariantCulture // Returns "Invariant Language (Invariant Country)"
CultureInfo.CurrentCulture // Returns "Lithuanian (Lithuania)"
CultureInfo.GetCultureInfo("en-US") // Returns "English (United States)"

There doesn't seem to be any linting involved here. So changing it to invariant will break GetCultureInfo, the other use cases just return invariant.

LucHeart commented 2 months ago

Wait shouldnt this

CultureInfo.GetCultureInfo("en-US") // Throws "System.Globalization.CultureNotFoundException: Only the invariant culture is supported in globalization-invariant mode. See https://aka.ms/GlobalizationInvariantMode for more information."

only happen when using <InvariantGlobalization>true</InvariantGlobalization> ? Seems like it is the other way around in your example, or am I reading that we wrong way around?

GedasFX commented 2 months ago

Wait shouldnt this

CultureInfo.GetCultureInfo("en-US") // Throws "System.Globalization.CultureNotFoundException: Only the invariant culture is supported in globalization-invariant mode. See https://aka.ms/GlobalizationInvariantMode for more information."

only happen when using <InvariantGlobalization>true</InvariantGlobalization> ? Seems like it is the other way around in your example, or am I reading that we wrong way around?

Yes, sorry, edited comment to fix the mistake. Yes it was backwards 🤦

GedasFX commented 2 months ago

Did one more quick test and yeah this property is taken from the EntryAssembly, meaning your bot.

I had this in a dependent project:

public static CultureInfo GetCultureInfo() => CultureInfo.GetCultureInfo("en-US");

And in the Entry project setting property to true changed the behavior of it. Honestly expected, but regardless, its not something Discord.NET can resolve. Best they can really do is try to use InvariantCulture in as many places as possible.

GedasFX commented 2 months ago

The root cause of the issue for me and OP seems to come from here:

https://github.com/discord-net/Discord.Net/blob/de8da0d3b998d32e248e5e438039d266139e4776/src/Discord.Net.WebSocket/Entities/Guilds/SocketGuild.cs#L563

Which ultimately comes from here: https://discord.com/developers/docs/resources/guild#guild-object

Discord announces the guild's preferred locale after it becomes available, and then we try to load the locale of it.

We could have an utility that loads locales (or returns invariant if this capability is not enabled in OS, however I do not know how to check for its enabled (outside of clearly jank implementations, such as this: https://stackoverflow.com/a/75299176)