Skip to content

Fix issue with force-reopen after 30 minutes #337

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.

Sign up for GitHub

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

Merged
nblumhardt merged 4 commits into from
Mar 7, 2025
Merged

Fix issue with force-reopen after 30 minutes #337

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c44444
Fix issue with force-reopen after 30 minutes
Falco20019 Feb 20, 2025
a3cd930
Fix issue with FileSink not closing the stream on hook errors
Falco20019 Feb 20, 2025
836854e
Ensure SelfLog will not tear into other tests
Falco20019 Feb 20, 2025
455a10f
Comment out SelfLog for now as requested
Falco20019 Feb 27, 2025
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
Prev Previous commit
Comment out SelfLog for now as requested
  • Loading branch information
@Falco20019
Falco20019 committed Feb 27, 2025
commit 455a10f76f73babf33fd9c097b7e5386b900cbae
2 changes: 2 additions & 0 deletions src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,12 @@ public void Emit(LogEvent logEvent)
AlignCurrentFileTo(now, nextSequence: true);
}

/* TODO: We REALLY should add this to avoid stuff become missing undetected.
if (_currentFile == null)
{
SelfLog.WriteLine("Log event {0} was lost since it was not possible to open the file or create a new one.", logEvent.RenderMessage());
}
Copy link
Member

@nblumhardt nblumhardt Feb 27, 2025

Choose a reason for hiding this comment

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

Not sure about doing this under the lock, since SelfLog may be writing somewhere slow. Might be safer to drop this for now, and reconsider separately.

Copy link
Contributor Author

@Falco20019 Falco20019 Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I can do that, I just had issues already that stuff was lost and we never found out why. But will remove it to progress with the PR for now. This should really never happen on regular use-cases and therefore should be neglectable and if the file was locked, we also use the SelfLog, so it was the nearest thing to consider :)

I will just comment it out so that it's easy to add in the future.

*/
}
}

Expand Down