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

AirBrake doesn't send Error()'s, only ErrorException()'s #23

Open
LinkExe opened this issue Nov 27, 2012 · 7 comments
Open

AirBrake doesn't send Error()'s, only ErrorException()'s #23

LinkExe opened this issue Nov 27, 2012 · 7 comments

Comments

@LinkExe
Copy link
Contributor

LinkExe commented Nov 27, 2012

Currently if you set a logger rule for "Error" as exampled below, and you use nLog.Error() instead of nLog.ErrorException(), Airbrake will not send an error report.

logger name="*" minlevel="Error" writeTo="airBrakeTarget"

Can we change this to allow either method to send the error?

Edit: Formatting

@Iristyle
Copy link
Owner

Yeah, I think this should probably be changed.

Presently it's checking for the existence of an Exception:
https://github.com/EastPoint/NLog.AirBrake/blob/master/src/NLog.AirBrake/AirBrakeTarget.cs#L42

But our own docs show setting a minlevel. I think by the time our target receives the Write call, NLog has already filtered based on what the minlevel in the config says.

So I think we should roll with that particular method -- as it's more consistent with how the other targets work.

So essentially we just have to remove the null check on the exception, and send all messages along.

/cc @DonMcNamara

@DonMcNamara
Copy link
Contributor

Agreed

@LinkExe
Copy link
Contributor Author

LinkExe commented Nov 27, 2012

If you remove the null check, you get a null error on the AirbrakeNotice notice = this.SharpbrakeClient.BuildNotice(logEvent.Exception);

Because nLog.Error() doesn't send the exception.

What I've been trying to do is just add an else or else if (logEvent.Exception == null), and build the notice with the AirbrakeError instead.

@Iristyle
Copy link
Owner

var notice = (logEvent.Exception != null) ? SharpBrakeClient.BuildNotice(logEvent.Exception) : SharpBrakeClient.BuildNotice();

Then add an overload here
https://github.com/EastPoint/NLog.AirBrake/blob/master/src/NLog.AirBrake/SharpbrakeClient.cs#L25

Or just modify the SharpbrakeClient wrapper there so that BuildNotice does a null check on Exception

@Iristyle
Copy link
Owner

/cc @Elindalyne ... fight it out and see who can fix it first! ;0

@Elindalyne
Copy link
Contributor

It looks like I'm going to have to make a few changes to the AirBrakeNoticeBuilder to allow messages to be sent without exceptions.

Is it acceptable to just send the formatted message and leave the Backtrace, ClassName and Catching Method empty?

@Iristyle
Copy link
Owner

Well.. I would just check to see if we can fulfill those pieces, and pass along as much info as possible.... if they're not there, they're not there. Simple null checks on LogEventInfo properties should be sufficient to determine that.

But yeah, feel free to mod @DonMcNamara code in the AirBrakeNoticeBuilder to accomplish that goal.... try not to mod the SharpBrake code where possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants