Fix https usage and invalid json for batches

datalust / seq-logging

A Node.js client for the Seq HTTP ingestion API

Apache License 2.0

25 stars 15 forks source link

Fix https usage and invalid json for batches #49

Closed Mairu closed 3 years ago

Mairu commented 3 years ago

https was is not working for me without that change
we have a scenario with nginx with mod security in front of seq which refuses to pass through the request because of the invalid json body

nblumhardt commented 3 years ago

Thanks! Surprising that the JSON bug has been lurking there for six years!

The test failures are due to some flakiness we're yet to flush out when running in GitHub actions - the change looks good to me :+1:

nblumhardt commented 3 years ago

The http vs https change is interesting - I've checked the documentation for request and the options it supports do include protocol:

https://nodejs.org/api/http.html#http_http_request_options_callback

Wonder whether we need to dig into this further?

Mairu commented 3 years ago

Thank you for that fast response.

For the protocol, the "problem" is, that you provide your own agent, and then the protocol of the agent and the request does not match. It seems that the agent ones is used in such cases.

nblumhardt commented 3 years ago

Thanks for the follow-up. We tested this out locally, and got a hard failure early on in the sending of the request (protocol not supported), so it doesn't appear that any HTTP request was being sent in the earlier/broken version. New build should have this all sorted out now :-) .. Thanks again!