octokit / octokit.js

The all-batteries-included GitHub SDK for Browsers, Node.js, and Deno.
MIT License
6.97k stars 1.02k forks source link

[BUG]: `log` option doesn't work, but the types suggest that it should #2571

Closed timrogers closed 10 months ago

timrogers commented 11 months ago

What happened?

The "Debug" section in the @octokit/rest.js docs states that you can pass the log: console option when instantiating the client and all requests will then be logged.

This works when you use rest.js directly, but it seems to be broken in the octokit package, even though the types included in octokit suggest that it should work:

Image

Any idea what's going on here?

I've pushed a minimal reproduction to my own GitHub repo, timrogers/octokit-js-logging-minimal-reproduction:

Versions

Octokit.js 3.1.1 Node v18.17.0

Relevant log output

N/A

Code of Conduct

github-actions[bot] commented 11 months ago

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

gr2m commented 11 months ago

It's not a bug. @octokit/rest loads a plugin for legacy compatibility: https://github.com/octokit/rest.js/blob/414943c79b7b5b94d31914c340957282c50b0312/src/index.ts#L2 that the octokit package does not. You can add the @octokit/plugin-request-log plugin if you like that, or implement your own request logging pretty easily using octokit.hook.wrap("request", ...): https://github.com/octokit/core.js/#hooks

timrogers commented 10 months ago

@gr2m Ah, super interesting! In that case, isn't it a bit strange to declare the log option in the types? I'll add that plugin now!

gr2m commented 10 months ago

isn't it a bit strange to declare the log option in the types

you can pass a custom logger to the Octokit constructor. By default it's just console, but I usually set it to a pino logger instance in most of my projects to get proper line-delimited JSON log output