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

Logging loop with NLog-Sentry #1741

Closed
mcliment opened this issue Jun 23, 2022 · 4 comments · Fixed by #1824
Closed

Logging loop with NLog-Sentry #1741

mcliment opened this issue Jun 23, 2022 · 4 comments · Fixed by #1824
Assignees
Labels
Bug Something isn't working NLog

Comments

@mcliment
Copy link

mcliment commented Jun 23, 2022

Package

Sentry.AspNetCore

.NET Flavor

.NET Core

.NET Version

6.0.2

OS

Windows

SDK Version

3.17.1

Self-Hosted Sentry Version

No response

Steps to Reproduce

From what I've read and what I see in the traces, seems that we are trying to log something when the HttpContext has been already disposed and that seems to happen on our app when a detached task fails (using Task.Run()) and tries to log the error.

The problem is not a simple exception that we could try to identify and fix (in this case, some bad data was sent to an API in the background task, a case we are fixing) but that it seems that NLog/Sentry enter in a loop that ends up exhausting app resources.

I attach some log files and traces to show the behavior.

One is a log file from NLog (there are a lot more errors but is essentially the same) and the other is a stack trace of the error from a memory dump.

stack_trace.txt
error_log.txt

Expected Result

The error to be logged just once. It's acceptable that there's some internal error accessing the context as at this point everything is more or less broken, but with a single log that we can see in Sentry/local log files is enough.

Actual Result

It seems that NLog tries to log the Sentry error trying to access a disposed object and then tries to send this error to Sentry again, ending up in a loop.

@SimonCropp
Copy link
Contributor

what version of NLog are you using? it sounds similar to this #1701 (comment) which was resolved in V5 of NLog NLog/NLog.Web#754

@SimonCropp SimonCropp moved this from Needs Discussion to Needs More Information in Mobile & Cross Platform SDK Jun 24, 2022
@SimonCropp
Copy link
Contributor

if not, can you provide a minimal repro as a console app

@mattjohnsonpint
Copy link
Contributor

So, it sounds like this isn't just for NLog, but if a diagnostic logger fails writing to its underlying logger, then we need to not log that failure, or we'll create an endless loop or stack overflow.

@mcliment
Copy link
Author

if not, can you provide a minimal repro as a console app

I'm trying to build a small project to reproduce the issue but it does not seem as simple as logging an exception in an unawaited task... there's something else that I'm trying to find. Let's see if I can come with something...

This was referenced Jul 28, 2022
@SimonCropp SimonCropp moved this from Needs Investigation to In Progress in Mobile & Cross Platform SDK Jul 28, 2022
Repository owner moved this from In Progress to Done in Mobile & Cross Platform SDK Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working NLog
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants