authStrategy does not inherit logger from Octokit

[gr2m]

What happened?

@octokit/auth-app introduced a new log option, similar to Octokit's logging via octokit/auth-app.js#190. But when I pass a custom logger as options.log to Octokit, it is not passed down to the configured authentication strategy.

Example:

const { Octokit } = require("@octokit/core")
const { createAppAuth } = require("@octokit/auth-app");

const octokit = new Octokit({
  log: myCustomLogger
  authStrategy: createAppAuth,
  auth: {
    id: 1,
    privateKey: PRIVATE_KEY,
    installationId: 2
  }
})

octokit.request('GET /repos/octokit/core.js')

If GitHub responds with a 401 (usually due to a replication lag), @octokit/auth-app is logging a warning:

[@octokit/auth-app] Retrying after 401 response to account for token replication delay (retry: 1, wait: 1s)

But it logs using the default console.warn.

Test case that reproduces the problem:
https://runkit.com/gr2m/octokit-core-authstrategy-custom-logger/1.0.0

What did you expect to happen?

I expected that myCustomLogger from the Octokit constructor options is passed down to createAppAuth({ log }), so that myCustomLogger.log.warn is used to log the warning instead of console.warn.

What the problem might be

We do pass the internal request method as options.request to createAppAuth(options) by default, see

core.js/src/index.ts

Lines 141 to 148 in 4582982

	const auth = options.authStrategy(
	Object.assign(
	{
	request: this.request,
	},
	options.auth
	)
	);

We have to do the same with the internal log object

How to contribute

Comment below if you would like to work on this issue. Then follow the instructions in our contributing guidelines

[ewanharris]

@gr2m can I claim this issue? Where would be the right place to add a test for this, log.test.ts or auth.test.ts?

[gr2m]

you can add the test to https://github.com/octokit/core.js/blob/master/test/issues.test.ts, that's exactly what we created this file for :)

[ewanharris]

Ah I just made the PR before I saw your comment. I'll move the test there and push a new commit

[gr2m]

no need, it's good as is 👍🏼

[github-actions]

🎉 This issue has been resolved in version 3.1.4 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀