Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): partially remove got #5596

Merged
merged 2 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"@logto/schemas": "workspace:^1.14.0",
"@logto/shared": "workspace:^3.1.0",
"@silverhand/essentials": "^2.9.0",
"@silverhand/slonik": "31.0.0-beta.2",
"@simplewebauthn/server": "^9.0.0",
"@withtyped/client": "^0.8.4",
"camelcase": "^8.0.0",
Expand Down Expand Up @@ -72,6 +73,7 @@
"koa-proxies": "^0.12.1",
"koa-router": "^12.0.0",
"koa-send": "^5.0.1",
"ky": "^1.2.3",
"lru-cache": "^10.0.0",
"nanoid": "^5.0.1",
"oidc-provider": "^8.2.2",
Expand All @@ -85,7 +87,6 @@
"roarr": "^7.11.0",
"samlify": "2.8.10",
"semver": "^7.3.8",
"@silverhand/slonik": "31.0.0-beta.2",
"snake-case": "^4.0.0",
"snakecase-keys": "^7.0.0",
"zod": "^3.22.4"
Expand Down Expand Up @@ -116,7 +117,7 @@
"jest-matcher-specific-error": "^1.0.0",
"jsonc-eslint-parser": "^2.4.0",
"lint-staged": "^15.0.0",
"nock": "^13.3.1",
"nock": "^14.0.0-beta.5",
gao-sun marked this conversation as resolved.
Show resolved Hide resolved
"node-mocks-http": "^1.12.1",
"nodemon": "^3.0.0",
"prettier": "^3.0.0",
Expand Down
32 changes: 17 additions & 15 deletions packages/core/src/libraries/cloud-connection.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type router from '@logto/cloud/routes';
import { cloudConnectionDataGuard, CloudScope } from '@logto/schemas';
import { formUrlEncodedHeaders } from '@logto/shared';
import { appendPath } from '@silverhand/essentials';
import Client from '@withtyped/client';
import { got } from 'got';
import ky from 'ky';
import { z } from 'zod';

import { EnvSet } from '#src/env-set/index.js';
Expand Down Expand Up @@ -71,20 +72,21 @@

const { tokenEndpoint, appId, appSecret, resource } = await this.getCloudConnectionData();

const httpResponse = await got.post({
url: tokenEndpoint,
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
Authorization: `Basic ${Buffer.from(`${appId}:${appSecret}`).toString('base64')}`,
},
form: {
grant_type: 'client_credentials',
resource,
scope: scopes.join(' '),
},
});

const result = accessTokenResponseGuard.safeParse(safeParseJson(httpResponse.body));
const text = await ky
.post(tokenEndpoint, {
headers: {
...formUrlEncodedHeaders,
Authorization: `Basic ${Buffer.from(`${appId}:${appSecret}`).toString('base64')}`,
},
body: new URLSearchParams({
grant_type: 'client_credentials',
resource,
scope: scopes.join(' '),
}),
})
.text();

const result = accessTokenResponseGuard.safeParse(safeParseJson(text));

if (!result.success) {
throw new Error('Unable to get access token for Cloud service');
Expand All @@ -107,7 +109,7 @@
const { endpoint } = await this.getCloudConnectionData();

this.client = new Client<typeof router>({
// TODO @sijie @darcy remove the 'api' appending in getCloudConnectionData()

Check warning on line 112 in packages/core/src/libraries/cloud-connection.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/core/src/libraries/cloud-connection.ts#L112

[no-warning-comments] Unexpected 'todo' comment: 'TODO @sijie @darcy remove the 'api'...'.
baseUrl: endpoint.replace('/api', ''),
headers: async () => {
return { Authorization: `Bearer ${await this.getAccessToken()}` };
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/libraries/hook/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ mockEsm('#src/utils/sign.js', () => ({
}));

const { sendWebhookRequest } = mockEsm('./utils.js', () => ({
sendWebhookRequest: jest.fn().mockResolvedValue({ statusCode: 200, body: '{"message":"ok"}' }),
sendWebhookRequest: jest
.fn()
.mockResolvedValue({ status: 200, text: async () => '{"message":"ok"}' }),
generateHookTestPayload,
parseResponse,
}));
Expand Down
12 changes: 7 additions & 5 deletions packages/core/src/libraries/hook/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
} from '@logto/schemas';
import { generateStandardId } from '@logto/shared';
import { conditional, pick, trySafe } from '@silverhand/essentials';
import { HTTPError } from 'got';
import { HTTPError } from 'ky';

import RequestError from '#src/errors/RequestError/index.js';
import { LogEntry } from '#src/middleware/koa-audit-log.js';
Expand Down Expand Up @@ -49,7 +49,7 @@
const {
applications: { findApplicationById },
logs: { insertLog },
// TODO: @gao should we use the library function thus we can pass full userinfo to the payload?

Check warning on line 52 in packages/core/src/libraries/hook/index.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/core/src/libraries/hook/index.ts#L52

[no-warning-comments] Unexpected 'todo' comment: 'TODO: @gao should we use the library...'.
users: { findUserById },
hooks: { findAllHooks, findHookById },
} = queries;
Expand Down Expand Up @@ -106,13 +106,15 @@
})
.then(async (response) => {
logEntry.append({
response: parseResponse(response),
response: await parseResponse(response),
});
})
.catch(async (error) => {
logEntry.append({
result: LogResult.Error,
response: conditional(error instanceof HTTPError && parseResponse(error.response)),
response: conditional(
error instanceof HTTPError && (await parseResponse(error.response))
),
error: conditional(error instanceof Error && String(error)),
});
});
Expand Down Expand Up @@ -151,8 +153,8 @@
code: 'hook.endpoint_responded_with_error',
},
{
responseStatus: error.response.statusCode,
responseBody: String(error.response.rawBody),
responseStatus: error.response.status,
responseBody: await error.response.text(),
} satisfies HookTestErrorResponseData
);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/libraries/hook/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { HookEvent } from '@logto/schemas';
import { createMockUtils } from '@logto/shared/esm';
import { got } from 'got';
import ky from 'ky';

const { jest } = import.meta;

const { mockEsm, mockEsmWithActual } = createMockUtils(jest);

const post = jest
.spyOn(got, 'post')
.spyOn(ky, 'post')
// @ts-expect-error
.mockImplementation(jest.fn(async () => ({ statusCode: 200, body: '{"message":"ok"}' })));

Expand Down Expand Up @@ -44,7 +44,7 @@ describe('sendWebhookRequest', () => {
},
json: testPayload,
retry: { limit: 3 },
timeout: { request: 10_000 },
timeout: 10_000,
});
});
});
19 changes: 11 additions & 8 deletions packages/core/src/libraries/hook/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@ import {
type HookConfig,
} from '@logto/schemas';
import { conditional, trySafe } from '@silverhand/essentials';
import { got, type Response } from 'got';
import ky, { type KyResponse } from 'ky';

import { sign } from '#src/utils/sign.js';

export const parseResponse = ({ statusCode, body }: Response) => ({
statusCode,
// eslint-disable-next-line no-restricted-syntax
body: trySafe(() => JSON.parse(String(body)) as unknown) ?? String(body),
});
export const parseResponse = async (response: KyResponse) => {
const body = await response.text();
return {
statusCode: response.status,
// eslint-disable-next-line no-restricted-syntax
body: trySafe(() => JSON.parse(body) as unknown) ?? String(body),
};
};

type SendWebhookRequest = {
hookConfig: HookConfig;
Expand All @@ -28,15 +31,15 @@ export const sendWebhookRequest = async ({
}: SendWebhookRequest) => {
const { url, headers, retries } = hookConfig;

return got.post(url, {
return ky.post(url, {
headers: {
'user-agent': 'Logto (https://logto.io/)',
...headers,
...conditional(signingKey && { 'logto-signature-sha-256': sign(signingKey, payload) }),
},
json: payload,
retry: { limit: retries ?? 3 },
timeout: { request: 10_000 },
timeout: 10_000,
});
};

Expand Down
5 changes: 2 additions & 3 deletions packages/integration-tests/src/api/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type Role,
type ProtectedAppMetadata,
} from '@logto/schemas';
import { formUrlEncodedHeaders } from '@logto/shared';
import { conditional } from '@silverhand/essentials';

import { authedAdminApi, oidcApi } from './api.js';
Expand Down Expand Up @@ -99,9 +100,7 @@ export const generateM2mLog = async (applicationId: string) => {

// This is a token request with insufficient parameters and should fail. We make the request to generate a log for the current machine to machine app.
return oidcApi.post('token', {
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
},
headers: formUrlEncodedHeaders,
body: new URLSearchParams({
client_id: id,
client_secret: secret,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('trigger hooks', () => {
expect(
logs.some(
({ payload: { result, error } }) =>
result === LogResult.Error && error === 'RequestError: Invalid URL'
result === LogResult.Error && error === 'TypeError: Failed to parse URL from not_work_url'
)
).toBeTruthy();

Expand Down Expand Up @@ -137,7 +137,7 @@ describe('trigger hooks', () => {

// Check hook trigger log
for (const [hook, expectedResult, expectedError] of [
[hook1, LogResult.Error, 'RequestError: Invalid URL'],
[hook1, LogResult.Error, 'TypeError: Failed to parse URL from not_work_url'],
[hook2, LogResult.Success, undefined],
[hook3, LogResult.Success, undefined],
] satisfies Array<[Hook, LogResult, Optional<string>]>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import assert from 'node:assert';
import { decodeAccessToken } from '@logto/js';
import { type LogtoConfig, Prompt, PersistKey } from '@logto/node';
import { GrantType, InteractionEvent, demoAppApplicationId } from '@logto/schemas';
import { formUrlEncodedHeaders } from '@logto/shared';
import { isKeyInObject, removeUndefinedKeys } from '@silverhand/essentials';
import { createRemoteJWKSet, jwtVerify } from 'jose';
import ky, { HTTPError } from 'ky';
Expand Down Expand Up @@ -51,9 +52,7 @@ class MockOrganizationClient extends MockClient {
try {
const json = await ky
.post(`${this.config.endpoint}/oidc/token`, {
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
},
headers: formUrlEncodedHeaders,
body: new URLSearchParams(
removeUndefinedKeys({
grant_type: GrantType.RefreshToken,
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/src/utils/fetch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** The necessary headers for sending a form-urlencoded request. */
export const formUrlEncodedHeaders = Object.freeze({
'content-type': 'application/x-www-form-urlencoded',
});
1 change: 1 addition & 0 deletions packages/shared/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ export * from './id.js';
export * from './user-display-name.js';
export * from './phone.js';
export * from './sub-domain.js';
export * from './fetch.js';
19 changes: 13 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.