Closed TedCha closed 1 month ago
Also, thanks for creating this library! It's great to have a loki client for pino.
Hey! Thanks for pointing out this issue
Issue is, got@12 is ESM only. I had plans to migrate pino-loki to ESM only but I'm afraid a lot of people are still stuck on CJS. So still not sure
And removing got
for something else would be a bit boring
So, since we have a workaround for this issue, I think we'll give ourselves some time to think about it
Ahh that makes sense. One option could be removing got
for fetch calls (though that might incur a similar issue of requiring Node 18+). I'd be happy to put in a PR for that if you agree (might take me a couple weeks as work/life takes priority).
Though as you said, there's a workaround so it might be best to wait and mull it over!
Yeah happy to accept a PR for removing got!
If you start working on it please lemme know. otherwise I will start the work when I have some time Thanks!
I started working on it! Give me a week or two and I'll create a PR for the fix.
Thank you!
I had some time tonight so I made the PR for the switch to fetch. I had to replace nock with msw due to the fact that nock doesn't support mocking native fetch calls š . I ran all the tests and everything looks like it's working. Let me know what you think.
Released with 2.3.1 !
Thanks again
Reproduction Code:
This code will result in the following
URI malformed
error (note: to see the error you'll have to use a valid loki domain,example.com
will result in 405 status code):The error is being caused by an invalid requestUrl being formed within
got
. In the scenario above, it will form the following url:There isn't any error in
pino-loki
library or even thegot
library. It's an error further downstream according to this issue: https://github.com/sindresorhus/got/issues/1169.Best course of action would be to upgrade the got package to v12.0.0 where the fix was introduced:
Workaround for anyone else who's experienced/experiencing this issue: