Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

Fixed errors not logging to SelfLog #284

Closed
wants to merge 1 commit into from
Closed

Fixed errors not logging to SelfLog #284

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Serilog.Sinks.Elasticsearch/Sinks/ElasticSearch/ElasticSearchSink.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ protected override void EmitBatch(IEnumerable<LogEvent> events)
var items = result.Body["items"];
foreach (var item in items)
{
if (item["index"] != null && HasProperty(item["index"], "error") && item["index"]["error"] != null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HasProperty function is not used now anymore. Would it be possible to include the ContainsKey functionality inside the HasProperty function instead? In order to make that one more robust?
Is there any way to test this BTW? So does it not break now for the other versions of EF?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. It should suffice to change the first condition of HasProperty from if (settings is System.Dynamic.ExpandoObject) to if (settings is System.Collections.IDictionary || settings is System.Dynamic.ExpandoObject).

I believe item isn't always an IDictionary, in fact I think I tried .ContainsKey first and it didn't work for me because it was some other kind of dynamic object.

No idea about how to integration test this, unfortunately. Unit testing the HasProperty function would be trivial of course but wouldn't help at all with ES version changes.

if (item["index"] != null && item["index"].ContainsKey("error") && item["index"]["error"] != null)
{
var e = events.ElementAt(indexer);
if (_state.Options.EmitEventFailure.HasFlag(EmitEventFailureHandling.WriteToSelfLog))
Expand Down