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

feat: enable allowH2 by default and require Node.js >= 18.20.0 #734

Merged
merged 7 commits into from
Nov 30, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
strategy:
fail-fast: false
matrix:
node-version: [18, 20, 22]
node-version: [18.20.0, 18, 20, 22]
os: [ubuntu-latest]

steps:
Expand Down Expand Up @@ -83,7 +83,7 @@ jobs:
strategy:
fail-fast: false
matrix:
node-version: [18, 20, 22]
node-version: [18.20.0, 18, 20, 22]
os: [ubuntu-latest]

steps:
Expand Down
27 changes: 27 additions & 0 deletions app/common/ErrorUtil.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const TimeoutErrorNames = [
'HttpClientRequestTimeoutError',
'HttpClientConnectTimeoutError',
'ConnectionError',
'ConnectTimeoutError',
'BodyTimeoutError',
'ResponseTimeoutError',
];

export function isTimeoutError(err: Error) {
if (TimeoutErrorNames.includes(err.name)) {
return true;
}

Check warning on line 13 in app/common/ErrorUtil.ts

View check run for this annotation

Codecov / codecov/patch

app/common/ErrorUtil.ts#L12-L13

Added lines #L12 - L13 were not covered by tests
if (err instanceof AggregateError && err.errors) {
for (const subError of err.errors) {
if (TimeoutErrorNames.includes(subError.name)) {
return true;
}
}
fengmk2 marked this conversation as resolved.
Show resolved Hide resolved
}

Check warning on line 20 in app/common/ErrorUtil.ts

View check run for this annotation

Codecov / codecov/patch

app/common/ErrorUtil.ts#L15-L20

Added lines #L15 - L20 were not covered by tests
if ('cause' in err && err.cause instanceof Error) {
if (TimeoutErrorNames.includes(err.cause.name)) {
return true;
}
}

Check warning on line 25 in app/common/ErrorUtil.ts

View check run for this annotation

Codecov / codecov/patch

app/common/ErrorUtil.ts#L22-L25

Added lines #L22 - L25 were not covered by tests
return false;
}
10 changes: 8 additions & 2 deletions app/common/adapter/NPMRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
HttpClientResponse,
} from 'egg';
import { PackageManifestType } from '../../repository/PackageRepository';
import { isTimeoutError } from '../ErrorUtil';

type HttpMethod = HttpClientRequestOptions['method'];

Expand Down Expand Up @@ -52,9 +53,14 @@
// large package: https://r.cnpmjs.org/%40procore%2Fcore-icons
// https://r.cnpmjs.org/intraactive-sdk-ui 44s
const authorization = this.genAuthorizationHeader(optionalConfig?.remoteAuthToken);
return await this.request('GET', url, undefined, { timeout: 120000, headers: { authorization } });
return await this.request('GET', url, undefined, {
timeout: 120000,
headers: { authorization },
});
} catch (err: any) {
if (err.name === 'ResponseTimeoutError') throw err;
if (isTimeoutError(err)) {
throw err;
}

Check warning on line 63 in app/common/adapter/NPMRegistry.ts

View check run for this annotation

Codecov / codecov/patch

app/common/adapter/NPMRegistry.ts#L62-L63

Added lines #L62 - L63 were not covered by tests
fengmk2 marked this conversation as resolved.
Show resolved Hide resolved
fengmk2 marked this conversation as resolved.
Show resolved Hide resolved
lastError = err;
}
retries--;
Expand Down
2 changes: 1 addition & 1 deletion app/common/adapter/binary/AbstractBinary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export abstract class AbstractBinary {
for (const version of versions) {
if (!version.modules) continue;
const modulesVersion = parseInt(version.modules);
// node v6.0.0 moduels 48 min
// node v6.0.0 modules 48 min
if (modulesVersion >= 48 && !nodeABIVersions.includes(modulesVersion)) {
nodeABIVersions.push(modulesVersion);
}
Expand Down
2 changes: 1 addition & 1 deletion app/common/adapter/changesStream/NpmChangesStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class NpmChangesStream extends AbstractChangeStream {
const db = this.getChangesStreamUrl(registry, since);
const { res } = await this.httpclient.request(db, {
streaming: true,
timeout: 10000,
timeout: 60000,
});

let buf = '';
Expand Down
15 changes: 7 additions & 8 deletions app/core/service/BinarySyncerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,19 @@ import {
EggHttpClient,
} from 'egg';
import fs from 'fs/promises';
import { sortBy } from 'lodash';
import binaries, { BinaryName, CategoryName } from '../../../config/binaries';
import { NFSAdapter } from '../../common/adapter/NFSAdapter';
import { TaskType, TaskState } from '../../common/enum/Task';
import { downloadToTempfile } from '../../common/FileUtil';
import { BinaryRepository } from '../../repository/BinaryRepository';
import { Task } from '../entity/Task';
import { Binary } from '../entity/Binary';
import { TaskService } from './TaskService';
import { NFSAdapter } from '../../common/adapter/NFSAdapter';
import { downloadToTempfile } from '../../common/FileUtil';
import { isTimeoutError } from '../../common/ErrorUtil';
import { AbstractBinary, BinaryItem } from '../../common/adapter/binary/AbstractBinary';
import { AbstractService } from '../../common/AbstractService';
import { BinaryType } from '../../common/enum/Binary';
import { sortBy } from 'lodash';
import { TaskType, TaskState } from '../../common/enum/Task';

function isoNow() {
return new Date().toISOString();
Expand Down Expand Up @@ -136,12 +137,10 @@ export class BinarySyncerService extends AbstractService {
this.logger.info('[BinarySyncerService.executeTask:success] taskId: %s, targetName: %s, log: %s, hasDownloadError: %s',
task.taskId, task.targetName, logUrl, hasDownloadError);
} catch (err: any) {
task.error = err.message;
task.error = `${err.name}: ${err.message}`;
logs.push(`[${isoNow()}] ❌ Synced "${binaryName}" fail, ${task.error}, log: ${logUrl}`);
logs.push(`[${isoNow()}] ❌❌❌❌❌ "${binaryName}" ❌❌❌❌❌`);
if (err.name === 'HttpClientRequestTimeoutError'
|| err.name === 'ConnectionError'
|| err.name === 'ConnectTimeoutError') {
if (isTimeoutError(err)) {
this.logger.warn('[BinarySyncerService.executeTask:fail] taskId: %s, targetName: %s, %s',
task.taskId, task.targetName, task.error);
this.logger.warn(err);
Expand Down
19 changes: 9 additions & 10 deletions app/core/service/ChangesStreamService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,21 @@ import {
EggObjectFactory,
Inject,
} from '@eggjs/tegg';
import { TaskState, TaskType } from '../../common/enum/Task';
import { AbstractService } from '../../common/AbstractService';
import { TaskRepository } from '../../repository/TaskRepository';
import { HOST_NAME, ChangesStreamTask, Task } from '../entity/Task';
import { E500 } from 'egg-errors';
import { PackageSyncerService, RegistryNotMatchError } from './PackageSyncerService';
import { TaskService } from './TaskService';
import { RegistryManagerService } from './RegistryManagerService';
import { E500 } from 'egg-errors';
import { ScopeManagerService } from './ScopeManagerService';
import { PackageRepository } from '../../repository/PackageRepository';
import { TaskRepository } from '../../repository/TaskRepository';
import { HOST_NAME, ChangesStreamTask, Task } from '../entity/Task';
import { Registry } from '../entity/Registry';
import { AbstractChangeStream } from '../../common/adapter/changesStream/AbstractChangesStream';
import { getScopeAndName } from '../../common/PackageUtil';
import { isTimeoutError } from '../../common/ErrorUtil';
import { GLOBAL_WORKER } from '../../common/constants';
import { ScopeManagerService } from './ScopeManagerService';
import { PackageRepository } from '../../repository/PackageRepository';
import { TaskState, TaskType } from '../../common/enum/Task';
import { AbstractService } from '../../common/AbstractService';

@SingletonProto({
accessLevel: AccessLevel.PUBLIC,
Expand Down Expand Up @@ -102,9 +103,7 @@ export class ChangesStreamService extends AbstractService {
}
} catch (err) {
this.logger.warn('[ChangesStreamService.executeTask:error] %s, exit now', err.message);
if (err.name === 'HttpClientRequestTimeoutError'
|| err.name === 'ConnectTimeoutError'
|| err.name === 'BodyTimeoutError') {
if (isTimeoutError(err)) {
this.logger.warn(err);
} else {
this.logger.error(err);
Expand Down
4 changes: 2 additions & 2 deletions app/port/schedule/SyncBinaryWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { IntervalParams, Schedule, ScheduleType } from '@eggjs/tegg/schedule';
import { Inject } from '@eggjs/tegg';
import { BinarySyncerService } from '../../core/service/BinarySyncerService';
import { isTimeoutError } from '../../common/ErrorUtil';

@Schedule<IntervalParams>({
type: ScheduleType.ALL,
Expand Down Expand Up @@ -35,8 +36,7 @@
const use = Date.now() - startTime;
this.logger.warn('[SyncBinaryWorker:executeTask:error] taskId: %s, targetName: %s, use %sms, error: %s',
task.taskId, task.targetName, use, err.message);
if (err.name === 'ConnectTimeoutError'
|| err.name === 'HttpClientRequestTimeoutError') {
if (isTimeoutError(err)) {

Check warning on line 39 in app/port/schedule/SyncBinaryWorker.ts

View check run for this annotation

Codecov / codecov/patch

app/port/schedule/SyncBinaryWorker.ts#L39

Added line #L39 was not covered by tests
this.logger.warn(err);
} else {
this.logger.error(err);
Expand Down
1 change: 1 addition & 0 deletions config/config.default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ export default (appInfo: EggAppConfig) => {

config.httpclient = {
useHttpClientNext: true,
allowH2: true,
};

config.view = {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,6 @@
},
"homepage": "https://github.com/cnpm/npmcore#readme",
"engines": {
"node": ">= 16.13.0"
"node": ">= 18.20.0"
}
}
18 changes: 16 additions & 2 deletions test/common/adapter/changesStream/NpmChangesStream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('test/common/adapter/changesStream/NpmChangesStream.test.ts', () => {
};
});
const res: ChangesStreamChange[] = [];
const stream = await npmChangesStream.fetchChanges(registry, '9517');
const stream = npmChangesStream.fetchChanges(registry, '9517');
for await (const change of stream) {
res.push(change);
}
Expand All @@ -74,7 +74,7 @@ describe('test/common/adapter/changesStream/NpmChangesStream.test.ts', () => {
};
});
const res: ChangesStreamChange[] = [];
const stream = await npmChangesStream.fetchChanges(registry, '9517');
const stream = npmChangesStream.fetchChanges(registry, '9517');
assert(stream);
rStream.push('{"seq":2');
rStream.push(',"id":"bac');
Expand All @@ -84,5 +84,19 @@ describe('test/common/adapter/changesStream/NpmChangesStream.test.ts', () => {
}
assert(res.length === 1);
});

it.skip('should read changes work', async () => {
for (let i = 0; i < 10000; i++) {
const stream = npmChangesStream.fetchChanges(registry, '36904024');
assert(stream);
try {
for await (const change of stream) {
console.log(change);
}
} catch (err) {
console.error(err);
}
}
});
fengmk2 marked this conversation as resolved.
Show resolved Hide resolved
});
});
4 changes: 2 additions & 2 deletions test/repository/TaskRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ describe('test/repository/TaskRepository.test.ts', () => {
const newData = EntityUtil.defaultData(data, 'taskId');
const task1 = new Task(newData);
const lastSince = new Date();
await setTimeout(1);
await setTimeout(100);
task1.updatedAt = lastSince;
await taskRepository.saveTask(task1);

assert(task1.updatedAt.getTime() > lastSince.getTime());
assert(task1.updatedAt.getTime() >= lastSince.getTime());
});
});

Expand Down
Loading