Skip to content

Commit

Permalink
feat: log option parameter (octokit#190)
Browse files Browse the repository at this point in the history
Co-authored-by: Roman Balayan <roman.balayan@paymaya.com>
  • Loading branch information
2 people authored and jefferycline1 committed Jul 5, 2021
1 parent 9586af9 commit 6d2d003
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 15 deletions.
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,25 @@ createAppAuth({
});
```

</td></tr>
<tr>
<th>
<code>log</code>
</th>
<th>
<code>object</code>
</th>
<td>
You can pass in your preferred logging tool by passing <code>option.log</code> to the constructor. If you would like to make the log level configurable using an environment variable or external option, we recommend the console-log-level package. For example:

```js
createAppAuth({
clientId: 123,
clientSecret: "secret",
log: require("console-log-level")({ level: "info" }),
});
```

</td></tr>
</tbody>
</table>
Expand Down
10 changes: 6 additions & 4 deletions src/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ export async function hook(
1000
);

console.warn(error.message);
console.warn(
state.log.warn(error.message);
state.log.warn(
`[@octokit/auth-app] GitHub API time and system time are different by ${diff} seconds. Retrying request with the difference accounted for.`
);

Expand All @@ -86,6 +86,7 @@ export async function hook(
endpoint.headers.authorization = `token ${token}`;

return sendRequestWithRetries(
state,
request,
endpoint as EndpointOptions,
createdAt
Expand All @@ -100,6 +101,7 @@ export async function hook(
* @see https://github.com/octokit/auth-app.js/issues/65
*/
async function sendRequestWithRetries(
state: State,
request: RequestInterface,
options: EndpointOptions,
createdAt: string,
Expand All @@ -126,13 +128,13 @@ async function sendRequestWithRetries(
++retries;

const awaitTime = retries * 1000;
console.warn(
state.log.warn(
`[@octokit/auth-app] Retrying after 401 response to account for token replication delay (retry: ${retries}, wait: ${
awaitTime / 1000
}s)`
);
await new Promise((resolve) => setTimeout(resolve, awaitTime));

return sendRequestWithRetries(request, options, createdAt, retries);
return sendRequestWithRetries(state, request, options, createdAt, retries);
}
}
10 changes: 9 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,15 @@ export const createAppAuth: StrategyInterface = function createAppAuth(
},
options.installationId
? { installationId: Number(options.installationId) }
: {}
: {},
{
log: Object.assign(
{
warn: console.warn.bind(console),
},
options.log
),
}
);

return Object.assign(auth.bind(null, state), {
Expand Down
8 changes: 8 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ export type StrategyOptions = {
request?: OctokitTypes.RequestInterface;
cache?: Cache;
timeDifference?: number;
log?: {
warn: (message: string, additionalInfo?: object) => any;
[key: string]: any;
};
};

export type StrategyOptionsWithDefaults = StrategyOptions & {
Expand Down Expand Up @@ -121,4 +125,8 @@ export type State = StrategyOptions & {
installationId?: number;
request: OctokitTypes.RequestInterface;
cache: Cache;
log: {
warn: (message: string, additionalInfo?: object) => any;
[key: string]: any;
};
};
85 changes: 75 additions & 10 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1243,9 +1243,12 @@ test("auth.hook(): handle 401 due to an exp timestamp in the past", async () =>
};
});

global.console.warn = jest.fn();

const auth = createAppAuth({
id: APP_ID,
privateKey: PRIVATE_KEY,
log: global.console,
});

const requestWithMock = request.defaults({
Expand All @@ -1259,8 +1262,6 @@ test("auth.hook(): handle 401 due to an exp timestamp in the past", async () =>
},
});

global.console.warn = jest.fn();

const promise = requestWithAuth("GET /app");

const { data } = await promise;
Expand Down Expand Up @@ -1315,9 +1316,12 @@ test("auth.hook(): handle 401 due to an exp timestamp in the past with 800 secon
};
});

global.console.warn = jest.fn();

const auth = createAppAuth({
id: APP_ID,
privateKey: PRIVATE_KEY,
log: global.console,
});

const requestWithMock = request.defaults({
Expand All @@ -1331,8 +1335,6 @@ test("auth.hook(): handle 401 due to an exp timestamp in the past with 800 secon
},
});

global.console.warn = jest.fn();

const promise = requestWithAuth("GET /app");

const { data } = await promise;
Expand Down Expand Up @@ -1385,9 +1387,12 @@ test("auth.hook(): handle 401 due to an iat timestamp in the future", async () =
};
});

global.console.warn = jest.fn();

const auth = createAppAuth({
id: APP_ID,
privateKey: PRIVATE_KEY,
log: global.console,
});

const requestWithMock = request.defaults({
Expand All @@ -1401,8 +1406,6 @@ test("auth.hook(): handle 401 due to an iat timestamp in the future", async () =
},
});

global.console.warn = jest.fn();

const promise = requestWithAuth("GET /app");
const { data } = await promise;

Expand Down Expand Up @@ -1440,9 +1443,12 @@ test("auth.hook(): throw 401 error in app auth flow without timing errors", asyn
},
});

global.console.warn = jest.fn();

const auth = createAppAuth({
id: APP_ID,
privateKey: PRIVATE_KEY,
log: global.console,
});

const requestWithMock = request.defaults({
Expand All @@ -1456,8 +1462,6 @@ test("auth.hook(): throw 401 error in app auth flow without timing errors", asyn
},
});

global.console.warn = jest.fn();

try {
await requestWithAuth("GET /app");
throw new Error("Should not resolve");
Expand Down Expand Up @@ -1518,10 +1522,13 @@ test("auth.hook(): handle 401 in first 5 seconds (#65)", async () => {
}
);

global.console.warn = jest.fn();

const auth = createAppAuth({
id: APP_ID,
privateKey: PRIVATE_KEY,
installationId: 123,
log: global.console,
});

const requestWithMock = request.defaults({
Expand All @@ -1538,8 +1545,6 @@ test("auth.hook(): handle 401 in first 5 seconds (#65)", async () => {
},
});

global.console.warn = jest.fn();

const promise = requestWithAuth("GET /repos/octocat/hello-world");

// it takes 3 retries until a total time of more than 5s pass
Expand Down Expand Up @@ -1831,3 +1836,63 @@ test("id and installationId can be passed as options", async () => {

expect(authentication.token).toEqual("secret123");
});

test("createAppAuth passed with log option", async () => {
const calls: String[] = [];

const mock = fetchMock
.sandbox()
.get("https://api.github.com/app", (_url, options) => {
const auth = (options.headers as { [key: string]: string | undefined })[
"authorization"
];
const [_, jwt] = (auth || "").split(" ");
const payload = JSON.parse(atob(jwt.split(".")[1]));
if (payload.exp < 600) {
return {
status: 401,
body: {
message:
"'Expiration time' claim ('exp') must be a numeric value representing the future time at which the assertion expires.",
documentation_url: "https://developer.github.com/v3",
},
headers: {
date: new Date(Date.now() + 30000).toUTCString(),
},
};
}

return {
status: 200,
body: [],
};
});

const auth = createAppAuth({
id: APP_ID,
privateKey: PRIVATE_KEY,
log: {
warn: () => calls.push("warn"),
},
});

const requestWithMock = request.defaults({
request: {
fetch: mock,
},
});
const requestWithAuth = requestWithMock.defaults({
request: {
hook: auth.hook,
},
});

const promise = requestWithAuth("GET /app");

const { data } = await promise;

expect(data).toStrictEqual([]);
expect(mock.done()).toBe(true);

expect(calls).toStrictEqual(["warn", "warn"]);
});

0 comments on commit 6d2d003

Please sign in to comment.