Aspnetcore: RequestLocalizationMiddleware logs at warning

[daniel-white]

Describe the bug

When the RequestLocalizationMiddleware detects an unsupported language, it logs at the warning level.

To Reproduce

Steps to reproduce the behavior:

1. Using this version of ASP.NET Core 2.1
2. Setup the localization as documented.
3. Set the culture/uiculture values to something that isn't added supported collections.
4. Observe warning error messages. See: RequestCultureProviderLoggerExtensions.cs

Expected behavior

This seems a bit severe given that this value is passed by the client, for instance if Spanish is requested but only French is specified, every requests gets 1 or more warnings.
Perhaps make this configurable and/or reduce it to a debug level.

[hishamco]

IMHO the WARN level is the suitable one for such scenario

[Eilon]

WARN seems high to me. I don't think you want something so trivial and inconsequential to cause warnings in the logs. I don't think it's even INFO - it should probably be whatever is 1 lower than INFO.

[Eilon]

That is, assuming that what's being discussed here is when a client sends an unsupported language (xx-YY) that a WARN log happens - that seems to much to me.

[daniel-white]

@Eilon yep its what the client is sending. most browsers send it automatically w/o configuration.

[daniel-white]

@Eilon would you take a PR for this to reduce this to Debug?

[sebastienros]

I think it's not just that it should be lowered to Debug. Because it will still log it even if the default culture is the one requested. It logs if the requested culture is not in the list of supported cultures, ignoring that the default culture might still be a match, and in this case it should not even log at all.

[daniel-white]

@sebastienros then that sounds like a bug/bigger change because the check to log is that it doesn't match anything.

[hishamco]

@Eilon can we make this configurable, something like:

services.AddLocalization(options => 
{
    options.ResourcesPath = "My/Resources");
    options.EnableLogging = true;
}

At least you can disable the logs by default

[Eilon]

I don't think we want an option for this. Less is more.

But we should make the built-in behavior be better because it seems the current behavior is not ideal.

I think in the meantime a PR to at least lower it to DEBUG would be a great first step.

[daniel-white]

@Eilon here's the commit when it went in. https://github.com/aspnet/AspNetCore/commit/41ae20525f4aa22490ffb435db2c86e2906a39b0 it mentions a PR that isn't part of this repo. how can i find the original issue?

[hishamco]

Here's the original repo https://github.com/aspnet/Localization

[daniel-white]

@hishamco it looks like it was your PR. care to share why WARN was used if you remember?

[hishamco]

Yep, the original issue came from a docs issue where the cookie value was c='fr-FR'|uic=''fr-FR I knew it is a typo to have a single quotes, but it will take time to discover the bug without a log

[daniel-white]

so maybe we should warn in the providers then if it is in an unexpected format. the Accept-Language header is standardized, so a non-matching language _shouldn't_ be a warning.

[hishamco]

so maybe we should warn in the providers

I don't think so, coz the provider executed once the middleware is registered, we need to validate on each request

[daniel-white]

@hishamco i'm proposing that the providers log in the request themselves in a method such as public ParseCookieValue(string value) for the cookie provider.

[hishamco]

FYI this issue can be fixed if we supporting custom cultures that are not pre-defined in options.SupportedCultures, please refer to this #2649

[mkArtakMSFT]

Thanks for contacting us, @daniel-white
We would happily consider a PR for this.

[hishamco]

@mkArtakMSFT we already fixed this to Debug here https://github.com/aspnet/Extensions/blob/master/src/Localization/Localization/src/Internal/ResourceManagerStringLocalizerLoggerExtensions.cs

[mkArtakMSFT]

Cool, thanks @hishamco!

[daniel-white]

@hishamco that commit isn't related. @mkArtakMSFT can we please reopen?
https://github.com/aspnet/AspNetCore/blob/master/src/Middleware/Localization/src/RequestCultureProviderLoggerExtensions.cs

[hishamco]

Sorry, It's my bad .. @daniel-white you can open a PR if you want, or I will do it in your behave

[mkArtakMSFT]

Thanks @hishamco !