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

Sink doesn't fail on 403, and other non-success codes #3

Open
jamesbascle opened this issue Apr 4, 2016 · 11 comments
Open

Sink doesn't fail on 403, and other non-success codes #3

jamesbascle opened this issue Apr 4, 2016 · 11 comments

Comments

@jamesbascle
Copy link
Contributor

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

var result = await httpClient.PostAsync(_logglyUrl, content);
result.EnsureSuccessStatusCode();

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.

@samirahmed
Copy link
Owner

@jamesbascle if you could submit a PR this would be great

@samirahmed
Copy link
Owner

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.

@cmcewen
Copy link

cmcewen commented Apr 11, 2016

I also have this issue and would appreciate immediate resolution

@crowell
Copy link

crowell commented Apr 11, 2016

👍 I depend heavily on this framework

@nambrot
Copy link

nambrot commented Apr 11, 2016

Me too

@jamesbascle
Copy link
Contributor Author

@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.

@jamesbascle
Copy link
Contributor Author

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.

@samirahmedsc
Copy link

@jamesbascle what is your nuget.org username? i added a jamesbascle as owner ... hopefully that is you

@jamesbascle
Copy link
Contributor Author

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.

@jamesbascle
Copy link
Contributor Author

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.

@jamesbascle
Copy link
Contributor Author

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.

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

6 participants