NickCraver / StackExchange.Exceptional

Error handler used for the Stack Exchange network
https://nickcraver.com/StackExchange.Exceptional/
Apache License 2.0
860 stars 170 forks source link

IP address is not saved #101

Closed StefanKert closed 6 years ago

StefanKert commented 6 years ago

As @nickalbrecht mentioned in #85 there is an issue with resolving the remote ip in the current alpha.

image

The Ip is always set to the default value 0.0.0.0. This is because of the new implementation for retrieving the Ip

https://github.com/NickCraver/StackExchange.Exceptional/blob/c4217546a80d15fb6cd6eae4e50e1b70e37364d1/src/StackExchange.Exceptional.Shared/Settings.cs#L61-L65

Basically, @NickCraver introduced this setting for getting the current IP address. This is then used in the Error class and since this setting is always null the ip address has no value in first place. When calling the Errors property IPAddress it is then loaded from ServerVariables which is not a real obvious behavior.

        public string IPAddress
        {
            get => _ipAddress ?? (_ipAddress = ServerVariables?.GetRemoteIP() ?? "");
            set => _ipAddress = value;
        }

So the first improvement that could be made, since the IPAddress is always logged, is to change the SetIPAddress method so that it directly calls GetRemoteIP if no Func is set.

Also this problem only exists in AspNetCore because the REMOTE_ADDR is never added to ServerVariables. If we add the the remote ip address we are able to save the ip address again.

  { "REMOTE_ADDR", context.Connection.RemoteIpAddress.ToString() }

image

NickCraver commented 6 years ago

Fixed in the commit above, but I'm not happy with the workaround and limitations to getting an IP. The settings split issue is the same as before (with GetCustomData), and will need more global love to make this more flexible and usable.