-
Notifications
You must be signed in to change notification settings - Fork 12
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
Sink doesn't fail on 403, and other non-success codes #3
Comments
@jamesbascle if you could submit a PR this would be great |
additionally i don't own a PC anymore so I can't really do anything with this code... sooooooo you might want to fork it make the changes and create your own nuget package. |
I also have this issue and would appreciate immediate resolution |
👍 I depend heavily on this framework |
Me too |
@samirahmed I have no problems taking over this thing...I'll fork and try to set up an Appveyor CI NuGet build this week. Will update this thread when done. |
Alright, I've got a CI build set up on AppVeyor, and it can push to NuGet.org. Currently have it set to Serilog.LogglyBulk. Would prefer to use the one @samirahmed set up if he can scrounge up the creds and XFer ownership/invite me as an Owner. |
@jamesbascle what is your nuget.org username? i added a jamesbascle as owner ... hopefully that is you |
It is indeed, but I don't see anything that has changed on NuGet.org from my side. I'll try pushing to it using a -prerelease package with 0 functional changes. |
Hmm. I am still unable to view any ownership capabilities in the NuGet.org site, nor is AppVeyor able to push to that feed using my API key. |
For all following the thread, I've released a new version with the suggested changes at https://www.nuget.org/packages/Serilog.LogglyBulk/0.1.2.1 You can see the code on my fork of this repo. Hopefully we can work out the issues with the original NuGet.org feed, and the repo can be XFerred to me, otherwise I'll maintain my fork and feed if any new issues come up. |
In the EmitBatchAsync method in LogglySink.cs, there is nothing that I could find that will cause the exception on line 42 to actually be caught, since HttpClient does not throw by default.
Perhaps we could change it to
To force an exception on non-success result codes? I noticed this while writing a local test and intentionally putting the wrong CustomerToken in, I did not get any output to Trace or Serilog.Debugging.SelfLog as I would expect on a 403.
Open to other fixes as well.
The text was updated successfully, but these errors were encountered: