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

Opening exception details tries to parse the field as XML rather than string for MSSQL #130

Closed
sommmen opened this issue Aug 12, 2024 · 10 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request v3

Comments

@sommmen
Copy link
Contributor

sommmen commented Aug 12, 2024

I'm seeing the following issue when opening exception details;

image

This is odd, the 'exception' detail field should be a plain string.

Sample FetchLogs;

{
    "logs": [
        {
            "rowNo": 1,
            "level": "Error",
            "message": "Exception whilst [REDACTED]",
            "timestamp": "2024-08-12T00:10:57Z",
            "propertyType": "xml",
            "exception": "Newtonsoft.Json.JsonReaderException: Bad JSON escape sequence: \\\u0026. Path \u0027base.originalUpload\u0027, line 1, position 75325.\r\n   at Newtonsoft.Json.JsonTextReader.ReadStringIntoBuffer(Char quote)\r\n   at Newtonsoft.Json.JsonTextReader.ParseValue()\r\n   at Newtonsoft.Json.Linq.JContainer.ReadContentFrom(JsonReader r, JsonLoadSettings settings)\r\n   at [REDACTED] in C:\\agent\\_work\\3\\s\\[REDACTED].cs:line 27\r\n   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader, Type objectType, Object existingValue)\r\n   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)\r\n   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)\r\n   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)\r\n   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)\r\n   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)\r\n   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)\r\n   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)\r\n   at [REDACTED] in C:\\agent\\_work\\3\\s\\[REDACTED].cs:line 1919",
            "properties": "\u003Cproperties\u003E\u003Cproperty key=\u0027OrderNumber\u0027\u003E10612ES\u003C/property\u003E\u003Cproperty key=\u0027SourceContext\u0027\u003E[REDACTED]\u003C/property\u003E\u003Cproperty key=\u0027OrderLineExternalId\u0027\u003E682632\u003C/property\u003E\u003Cproperty key=\u0027OrderExternalId\u0027\u003E809812\u003C/property\u003E\u003Cproperty key=\u0027DomainShortName\u0027\u003EES\u003C/property\u003E\u003Cproperty key=\u0027JobId\u0027\u003E63194\u003C/property\u003E\u003Cproperty key=\u0027CreatedAt\u0027\u003E08/11/2024 20:03:59\u003C/property\u003E\u003C/properties\u003E"
        }
    ],
    "total": 140499,
    "count": 1,
    "currentPage": 1
}
@followynne
Copy link
Member

@sommmen

Can you kindly provide me additional context?

As you see, the property type is XML in the sample, thus the Provider you're using expect the data serialization as XML (I guess you're using the MSSQL Provider :D)

Please let me know

  • provider you use
  • the sink configuration (if possible)
  • any useful additional info, if possible

Addendum: I'll also change the implementation to simply print out the string data, if code parsing fails 👍

@followynne followynne self-assigned this Aug 12, 2024
@followynne followynne added bug Something isn't working enhancement New feature or request v3 labels Aug 12, 2024
@followynne followynne changed the title Openinging exception details tries to parse the field as xml rather than a string Opening exception details tries to parse the field as XML rather than string Aug 12, 2024
@sommmen
Copy link
Contributor Author

sommmen commented Aug 12, 2024

@sommmen

Can you kindly provide me additional context?

As you see, the property type is XML in the sample, thus the Provider you're using expect the data serialization as XML (I guess you're using the MSSQL Provider :D)

Please let me know

* provider you use

* the sink configuration (if possible)

* any useful additional info, if possible

Addendum: I'll also change the implementation to simply print out the string data, if code parsing fails 👍

"As you see, the property type is XML in the sample"
I'm confused, the $.logs.exception field of the json i provided is not xml and it is a plain string field (the output of Exception.ToString()). The exception holds no xml or json.

nvm, i see what you mean now $.logs.propertyType is XML. For the MSSQL sink the exception field is a plain string and not XML.

Could this be the same confusion we had in #109 (comment) ?
My config is pretty much the default for the serilog mssql sink.

Startup.cs

services.AddSerilogUi(options =>
{
    options.UseSqlServer(c =>
    {
        c.WithConnectionString(_defaultConnectionString);
        c.WithSchema("AppLog");
        c.WithTable("WebApi");
        c.WithCustomProviderName("Web");
    });

    options.UseSqlServer(c =>
    {
        c.WithConnectionString(_defaultConnectionString);
        c.WithSchema("AppLog");
        c.WithTable("AuthApi");
        c.WithCustomProviderName("Auth");
    });

    options.UseSqlServer(c =>
    {
        c.WithConnectionString(_defaultConnectionString);
        c.WithSchema("AppLog");
        c.WithTable("Hangfire");
        c.WithCustomProviderName("Hangfire");
    });

    options.AddScopedSyncAuthFilter<MyUiAuthorizationFilter>();
});

...

app.UseSerilogUi(options =>
{
    options.WithHomeUrl(env.IsDevelopment() ? "http://localhost:4200" : "https://opg.systems");
    options.InjectJavascript("/serilog-ui/custom.js", injectInHead: true);
    options.WithAuthenticationType(AuthenticationType.Jwt);
});

Serilog config of one of the apps (there is a common appsettings.json file, hence the :2 which just means another array entry)

"Serilog": {
  "Using:2": "Serilog.Sinks.MSSqlServer",
  "WriteTo:2": {
    "Name": "MSSqlServer",
    "Args": {
      "connectionString": "AppLogConnection",
      "sinkOptionsSection": {
        "tableName": "WebApi",
        "schemaName": "AppLog",
        "autoCreateSqlTable": true
      }
    }
  },
  "Filter": [
    {
      "Name": "ByExcluding",
      "Args": {
        "expression": "(RequestPath = '/api/maat' and (RequestMethod = 'POST' or RequestMethod = 'OPTIONS')) or RequestPath = '/api/health/ping' or StartsWith(RequestPath, '/swagger') or StartsWith(RequestPath, '/hangfire') or StartsWith(RequestPath, '/serilog-ui')"
      }
    }
  ]
}

@sommmen sommmen changed the title Opening exception details tries to parse the field as XML rather than string Opening exception details tries to parse the field as XML rather than string for MSSQL Aug 12, 2024
@followynne
Copy link
Member

followynne commented Aug 12, 2024

@sommmen

I think you're right - I'll give a try with the sample I made in the repo and try to understand how to improve this point :)

(something related: https://github.com/serilog-mssql/serilog-sinks-mssqlserver?tab=readme-ov-file#properties)

I'll notify you as soon as I have news!

followynne added a commit to followynne/serilog-ui that referenced this issue Aug 14, 2024
@followynne
Copy link
Member

Released in v3.0.1.

In the upcoming v3.1.0, Exception column rendering will be improved :)

@sommmen
Copy link
Contributor Author

sommmen commented Aug 15, 2024

Released in v3.0.1.

In the upcoming v3.1.0, Exception column rendering will be improved :)

Will that be released momentarily or after a while?

@followynne
Copy link
Member

@sommmen

The feature is ready but I wanted to see if I can develop a second feature for v3.1, before releasing 😊

If you want to try it, I can create a v3.1 beta for UI and MSSQL provider - let me know if you want it and I'll do it later today 😁

@sommmen
Copy link
Contributor Author

sommmen commented Aug 15, 2024

@sommmen

The feature is ready but I wanted to see if I can develop a second feature for v3.1, before releasing 😊

If you want to try it, I can create a v3.1 beta for UI and MSSQL provider - let me know if you want it and I'll do it later today 😁

If the release will be later than Monday please publish a beta package, else I don't mind waiting. It has to do with me doing daily checkups.

@followynne
Copy link
Member

followynne commented Aug 15, 2024

@sommmen

gotcha, you have UI and MSSQL v3.1.0-beta.1 to test out 😉 feel free to share feedback about it!

I prefer to take additional time for the stable release, 'cause I want to try to add the Log Database Id to the viewer in this release!

@sommmen
Copy link
Contributor Author

sommmen commented Aug 15, 2024

@sommmen

gotcha, you have UI and MSSQL v3.1.0-beta.1 to test out 😉 feel free to share feedback about it!

I prefer to take additional time for the stable release, 'cause I want to try to add the Log Database Id to the viewer in this release!

@followynne thanks - this works great, thank you!
I've checked the other improvements aswell and they work great.

Only nitpick i have is that the exception info is now very dark and hard to read;
Aside from that the modal feels a bit small - it could be a little bit bigger imo.
The cursor has the blocked symbol, but that also feels a bit off.

image

@followynne
Copy link
Member

@sommmen

Thanks for the feedback! I still have the pr open, I'll change something 😁
the cursor is due to the copy button on the right!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request v3
Projects
None yet
Development

No branches or pull requests

2 participants