search

logdna / logger-node

A nodejs logger client for LogDNA
MIT License
34 stars 17 forks source link

Don't set 'user-agent' header when running in browser #22

Closed geraintguan closed 4 years ago

geraintguan commented 4 years ago

Hi! 👋

I'm attempting to use your client in the browser but I'm running into an issue where the logs are failing to be sent due to the client attempting to set the user-agent header (which is the same issue raised in #21).

I've forked the client and published that version to workaround this issue for now; however it would be great if this fix was merged upstream so I don't have to maintain a forked version.

My fix conditionally sets the user-agent header only if the client isn't running in the browser (which I detect by simply checking if the window variable is undefined).

Feel free to modify this code if you'd prefer to have your own version of the fix (it's a fairly straightforward change) or close this if you'd prefer a different solution, just thought I'd contribute my solution in case it's helpful!

darinspivey commented 4 years ago

Thanks for all of the details! We will have a fix up shortly, although I think I'll open another PR for it. I'd like to keep browser-specific "code" out there and have it be more explicit. This is an edge case, so I think if someone like yourself is taking additional steps to get it to work in a browser, then an instantiation option such as fromBrowser: true would make more sense. This would make testing easier, and allow the client to take action for a browser rather than to magically figure out if one is in use.

I'm going to run this by the team, but we should have a fix up soon.

darinspivey commented 4 years ago

Thank you for the suggestion, but we have chosen to fix this on the server side and allow the user agent header for CORS. The fix is merged, and we will be releasing it in the next cycle. Please track the open issue for when that is complete. Thanks!