feat: proxy mode #571
feat: proxy mode #571
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #571 +/- ##
==========================================
- Coverage 96.83% 96.68% -0.16%
==========================================
Files 180 187 +7
Lines 17639 18436 +797
Branches 2295 2420 +125
==========================================
+ Hits 17080 17824 +744
- Misses 559 612 +53 ☔ View full report in Codecov by Sentry. |
|
目前代理模式无法配置上游仓库认证头的信息，认证头应该作为仓库的一个属性记录在数据库里，可以通过接口修改。 |
|
@hezhengxu2018 🤩 已经改完了吗？ |
是的，现在会定时更新已缓存的manifest，不过还没有手动管理缓存的接口。verdicca和nexus都没做这个，先基础功能确定没问题再做吧 |
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
管理缓存的接口也加上了，功能相对来说比较完整了，帮忙看一下吧 @elrrrrrrr @fengmk2 |
🤩 改动内容比较多 我明天详细看下 🙏🏻 |
| pkg = await this.getPackageEntityByFullname(fullname, allowSync); | ||
| packageVersion = await this.getPackageVersionEntity(pkg, version, allowSync); | ||
| } catch (error) { | ||
| if (this.config.cnpmcore.syncMode === SyncMode.proxy) { |
There was a problem hiding this comment.
这里直接用 AOP 来组织一下逻辑会清晰一下，类似 EventCorkAdvice 的实现
在原来方法返回前，判断一下是否有返回值，如果没有的话再去触发 proxyMode 的 fallback 逻辑
There was a problem hiding this comment.
尝试了一下好像没有改写函数返回的切点？这样的话确实可以较为优雅的处理代理的逻辑但是改不了接口的返回值。用户还是会收到not found的返回。还是希望代理模式的表现和nexus与verdaccio保持一致。
|
proxyMode 定位是做代理模式，然后本地 registry 做缓存加速？需要确认一下 manifest 以本地为准还是以上游为准。 目前实现是 manifest 和 tgz 请求时，将代理结果直接返回，并创建同步任务
如果 proxyMode 中直接命中 tgz 下载，需要等异步定时任务补偿后才能继续访问，否则 manifest 会返回单个版本？客户单查包信息的时候就过期了。 我们是否可以改为 proxyMode 下，始终返回上游 registry 信息已获取更高的实时性。 |
开启proxyMode之后返回的manifest都是上游仓库上次更新的manifest，不会使用数据库里的manifest信息。如果上游仓库无法正常使用了，切回到none模式下才会返回代理仓库已缓存版本的manifest。如果异步任务还没同步完成会一直通过反向代理返回上游仓库的tgz信息，用户不会需要等待异步任务完成，当异步任务完成后才会优先从对象存储中读取tgz。 代理仓库主要的功能点是在访问外网或者官方npm仓库非常缓慢甚至无法访问的情况下能够缓存结果加速内网用户安装依赖，同时即使外网无法访问了也不影响内网用户照常使用已经缓存的依赖，所以始终返回上游仓库可能不行，如果内网用户发现代理仓库的缓存需要更新或者缓存脏数据了可以使用 |
|
manifest 以本地为准，因为nexus是这么做的。代理模式整体是一个缓存，如果网络不好还一直使用上游仓库的索引的话就没有缓存的意义了，不过nexus默认刷新manifest的频率很高，30分钟就会去刷新一次，我设置每天刷新一次感觉有点保守了。 |
hezhengxu2018
force-pushed
the
feat/proxy-mode
branch
from
e5627bf
to
68b26f7
Compare
|
Important Auto Review SkippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a robust proxy caching system for package manifests, enhancing the efficiency of data retrieval and management. Key changes include the refactoring of methods for clarity, the introduction of new classes and enums for proxy cache handling, and updates to configurations and error handling to support new sync modes. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| describe('getSourceManifestAndCache()', () => { | ||
| it('should get full package manifest', async () => { | ||
| const data = await TestUtil.readJSONFile( | ||
| TestUtil.getFixtures('registry.npmjs.org/foobar.json'), | ||
| ); | ||
| mock(npmRegistry, 'getFullManifests', async () => { | ||
| return { | ||
| status: 200, | ||
| data, | ||
| }; | ||
| }); | ||
| const { manifest } = await proxyCacheService.getSourceManifestAndCache( | ||
| 'foobar', | ||
| DIST_NAMES.FULL_MANIFESTS, | ||
| ); | ||
| const versionArr = Object.values(manifest.versions); | ||
| for (const i of versionArr) { | ||
| assert(i.dist.tarball.includes('http://localhost:7001')); | ||
| } | ||
| }); | ||
|
|
||
| it('should get abbreviated package manifest', async () => { | ||
| const data = await TestUtil.readJSONFile( | ||
| TestUtil.getFixtures('registry.npmjs.org/abbreviated_foobar.json'), | ||
| ); | ||
| mock(npmRegistry, 'getAbbreviatedManifests', async () => { | ||
| return { | ||
| status: 200, | ||
| data, | ||
| }; | ||
| }); | ||
| const { manifest } = await proxyCacheService.getSourceManifestAndCache( | ||
| 'foobar', | ||
| DIST_NAMES.ABBREVIATED_MANIFESTS, | ||
| ); | ||
| const versionArr = Object.values(manifest.versions); | ||
| for (const i of versionArr) { | ||
| assert(i.dist.tarball.includes('http://localhost:7001')); | ||
| } | ||
| }); | ||
|
|
||
| it('should get full package version manifest', async () => { | ||
| const data = await TestUtil.readJSONFile( | ||
| TestUtil.getFixtures('registry.npmjs.org/foobar/1.0.0/package.json'), | ||
| ); | ||
| mock(npmRegistry, 'getPackageVersionManifest', async () => { | ||
| return { | ||
| status: 200, | ||
| data, | ||
| }; | ||
| }); | ||
| const { manifest } = await proxyCacheService.getSourceManifestAndCache( | ||
| 'foobar', | ||
| DIST_NAMES.MANIFEST, | ||
| '1.0.0', | ||
| ); | ||
| assert(manifest.dist); | ||
| assert(manifest.dist.tarball.includes('http://localhost:7001')); | ||
| }); | ||
|
|
||
| it('should get abbreviated package version manifest', async () => { | ||
| const data = await TestUtil.readJSONFile( | ||
| TestUtil.getFixtures( | ||
| 'registry.npmjs.org/foobar/1.0.0/abbreviated.json', | ||
| ), | ||
| ); | ||
| mock(npmRegistry, 'getAbbreviatedPackageVersionManifest', async () => { | ||
| return { | ||
| status: 200, | ||
| data, | ||
| }; | ||
| }); | ||
| const { manifest } = await proxyCacheService.getSourceManifestAndCache( | ||
| 'foobar', | ||
| DIST_NAMES.ABBREVIATED, | ||
| '1.0.0', | ||
| ); | ||
| assert(manifest.dist); | ||
| assert(manifest.dist.tarball.includes('http://localhost:7001')); | ||
| }); |
There was a problem hiding this comment.
Tip
Codebase Verification
The tests for the getSourceManifestAndCache() method in ProxyCacheService.test.ts do not cover scenarios where the upstream registry returns errors or non-200 status codes. This is confirmed by the absence of any test cases handling status !== 200 in the test file.
Analysis chain
The tests for getSourceManifestAndCache() method effectively cover different manifest types. Ensure that the tests also cover the scenarios where the upstream registry returns errors or non-200 status codes.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Ensure error handling is tested for non-200 responses in getSourceManifestAndCache
rg --type ts "getSourceManifestAndCache" -A 50 | grep "status !== 200"
Length of output: 149
Script:
#!/bin/bash
# Search for test cases handling non-200 status codes in ProxyCacheService.test.ts
rg --type ts "status !== 200" -- "test/core/service/ProxyCacheService.test.ts"
Length of output: 78
| async getPackageManifest(fullname: string, fileType: DIST_NAMES.FULL_MANIFESTS| DIST_NAMES.ABBREVIATED_MANIFESTS): Promise<AbbreviatedPackageManifestType|PackageManifestType> { | ||
| const cachedStoreKey = (await this.proxyCacheRepository.findProxyCache(fullname, fileType))?.filePath; | ||
| if (cachedStoreKey) { | ||
| const nfsBytes = await this.nfsAdapter.getBytes(cachedStoreKey); | ||
| if (nfsBytes) { | ||
| let nfsPkgManifgest; | ||
| try { | ||
| const nfsString = Buffer.from(nfsBytes).toString(); | ||
| nfsPkgManifgest = JSON.parse(nfsString); | ||
| } catch { | ||
| // JSON parse error | ||
| await this.nfsAdapter.remove(cachedStoreKey); | ||
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType); | ||
| throw new InternalServerError('manifest JSON in NFS parse error'); | ||
| } | ||
| return nfsPkgManifgest; | ||
| } | ||
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType); | ||
| throw new NotFoundError('can not found manifest in NFS.'); | ||
| } | ||
|
|
||
| const { manifest } = await this.getSourceManifestAndCache<typeof fileType>(fullname, fileType); | ||
| this.backgroundTaskHelper.run(async () => { | ||
| const cachedFiles = ProxyCache.create({ fullname, fileType }); | ||
| await this.proxyCacheRepository.saveProxyCache(cachedFiles); | ||
| }); | ||
| return manifest; | ||
| } |
There was a problem hiding this comment.
The method getPackageManifest correctly handles caching and fetching from NFS. However, consider adding more robust error handling for NFS operations and JSON parsing.
- throw new InternalServerError('manifest JSON in NFS parse error');
+ throw new InternalServerError('Failed to parse manifest JSON from NFS. Please check the data integrity.');Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async getPackageManifest(fullname: string, fileType: DIST_NAMES.FULL_MANIFESTS| DIST_NAMES.ABBREVIATED_MANIFESTS): Promise<AbbreviatedPackageManifestType|PackageManifestType> { | |
| const cachedStoreKey = (await this.proxyCacheRepository.findProxyCache(fullname, fileType))?.filePath; | |
| if (cachedStoreKey) { | |
| const nfsBytes = await this.nfsAdapter.getBytes(cachedStoreKey); | |
| if (nfsBytes) { | |
| let nfsPkgManifgest; | |
| try { | |
| const nfsString = Buffer.from(nfsBytes).toString(); | |
| nfsPkgManifgest = JSON.parse(nfsString); | |
| } catch { | |
| // JSON parse error | |
| await this.nfsAdapter.remove(cachedStoreKey); | |
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType); | |
| throw new InternalServerError('manifest JSON in NFS parse error'); | |
| } | |
| return nfsPkgManifgest; | |
| } | |
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType); | |
| throw new NotFoundError('can not found manifest in NFS.'); | |
| } | |
| const { manifest } = await this.getSourceManifestAndCache<typeof fileType>(fullname, fileType); | |
| this.backgroundTaskHelper.run(async () => { | |
| const cachedFiles = ProxyCache.create({ fullname, fileType }); | |
| await this.proxyCacheRepository.saveProxyCache(cachedFiles); | |
| }); | |
| return manifest; | |
| } | |
| async getPackageManifest(fullname: string, fileType: DIST_NAMES.FULL_MANIFESTS| DIST_NAMES.ABBREVIATED_MANIFESTS): Promise<AbbreviatedPackageManifestType|PackageManifestType> { | |
| const cachedStoreKey = (await this.proxyCacheRepository.findProxyCache(fullname, fileType))?.filePath; | |
| if (cachedStoreKey) { | |
| const nfsBytes = await this.nfsAdapter.getBytes(cachedStoreKey); | |
| if (nfsBytes) { | |
| let nfsPkgManifgest; | |
| try { | |
| const nfsString = Buffer.from(nfsBytes).toString(); | |
| nfsPkgManifgest = JSON.parse(nfsString); | |
| } catch { | |
| // JSON parse error | |
| await this.nfsAdapter.remove(cachedStoreKey); | |
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType); | |
| throw new InternalServerError('Failed to parse manifest JSON from NFS. Please check the data integrity.'); | |
| } | |
| return nfsPkgManifgest; | |
| } | |
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType); | |
| throw new NotFoundError('can not found manifest in NFS.'); | |
| } | |
| const { manifest } = await this.getSourceManifestAndCache<typeof fileType>(fullname, fileType); | |
| this.backgroundTaskHelper.run(async () => { | |
| const cachedFiles = ProxyCache.create({ fullname, fileType }); | |
| await this.proxyCacheRepository.saveProxyCache(cachedFiles); | |
| }); | |
| return manifest; | |
| } |
| async removeProxyCache(fullname: string, fileType: DIST_NAMES, version?: string) { | ||
| const storeKey = isPkgManifest(fileType) | ||
| ? `/${PROXY_CACHE_DIR_NAME}/${fullname}/${fileType}` | ||
| : `/${PROXY_CACHE_DIR_NAME}/${fullname}/${version}/${fileType}`; | ||
| await this.nfsAdapter.remove(storeKey); | ||
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType, version); | ||
| } |
There was a problem hiding this comment.
The method removeProxyCache effectively handles the removal of cache entries. Consider adding logging for successful removals for better traceability.
+ this.logger.info(`Successfully removed cache for ${fullname} with fileType ${fileType}`);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async removeProxyCache(fullname: string, fileType: DIST_NAMES, version?: string) { | |
| const storeKey = isPkgManifest(fileType) | |
| ? `/${PROXY_CACHE_DIR_NAME}/${fullname}/${fileType}` | |
| : `/${PROXY_CACHE_DIR_NAME}/${fullname}/${version}/${fileType}`; | |
| await this.nfsAdapter.remove(storeKey); | |
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType, version); | |
| } | |
| async removeProxyCache(fullname: string, fileType: DIST_NAMES, version?: string) { | |
| const storeKey = isPkgManifest(fileType) | |
| ? `/${PROXY_CACHE_DIR_NAME}/${fullname}/${fileType}` | |
| : `/${PROXY_CACHE_DIR_NAME}/${fullname}/${version}/${fileType}`; | |
| await this.nfsAdapter.remove(storeKey); | |
| await this.proxyCacheRepository.removeProxyCache(fullname, fileType, version); | |
| this.logger.info(`Successfully removed cache for ${fullname} with fileType ${fileType}`); | |
| } |
| async createTask(targetName: string, options: UpdateProxyCacheTaskOptions): Promise<CreateUpdateProxyCacheTask> { | ||
| return await this.taskService.createTask(Task.createUpdateProxyCache(targetName, options), false) as CreateUpdateProxyCacheTask; | ||
| } | ||
|
|
||
| async findExecuteTask() { | ||
| return await this.taskService.findExecuteTask(TaskType.UpdateProxyCache); | ||
| } | ||
|
|
||
| async executeTask(task: Task) { | ||
| const logs: string[] = []; | ||
| const fullname = (task as CreateUpdateProxyCacheTask).data.fullname; | ||
| const { fileType, version } = (task as CreateUpdateProxyCacheTask).data; | ||
| let cacheBytes; | ||
| logs.push(`[${isoNow()}] 🚧🚧🚧🚧🚧 Start update "${fullname}-${fileType}" 🚧🚧🚧🚧🚧`); | ||
| try { | ||
| if (isPkgManifest(fileType)) { | ||
| const cachedFiles = await this.proxyCacheRepository.findProxyCache(fullname, fileType); | ||
| if (!cachedFiles) throw new Error('task params error, can not found record in repo.'); | ||
| cacheBytes = (await this.getSourceManifestAndCache<typeof fileType>(fullname, fileType)).proxyBytes; | ||
| ProxyCache.update(cachedFiles); | ||
| await this.proxyCacheRepository.saveProxyCache(cachedFiles); | ||
| } else { | ||
| task.error = 'Unacceptable file type.'; | ||
| logs.push(`[${isoNow()}] ❌ ${task.error}`); | ||
| logs.push(`[${isoNow()}] ❌❌❌❌❌ ${fullname}-${fileType} ${version ?? ''} ❌❌❌❌❌`); | ||
| await this.taskService.finishTask(task, TaskState.Fail, logs.join('\n')); | ||
| this.logger.info('[ProxyCacheService.executeTask:fail] taskId: %s, targetName: %s, %s', | ||
| task.taskId, task.targetName, task.error); | ||
| return; | ||
| } | ||
| } catch (error) { | ||
| task.error = error; | ||
| logs.push(`[${isoNow()}] ❌ ${task.error}`); | ||
| logs.push(`[${isoNow()}] ❌❌❌❌❌ ${fullname}-${fileType} ${version ?? ''} ❌❌❌❌❌`); | ||
| await this.taskService.finishTask(task, TaskState.Fail, logs.join('\n')); | ||
| this.logger.info('[ProxyCacheService.executeTask:fail] taskId: %s, targetName: %s, %s', | ||
| task.taskId, task.targetName, task.error); | ||
| return; | ||
| } | ||
| logs.push(`[${isoNow()}] 🟢 Update Success.`); | ||
| const isFullManifests = fileType === DIST_NAMES.FULL_MANIFESTS; | ||
| const cachedKey = await this.cacheService.getPackageEtag(fullname, isFullManifests); | ||
| if (cachedKey) { | ||
| const { shasum: etag } = await calculateIntegrity(cacheBytes); | ||
| await this.cacheService.savePackageEtagAndManifests(fullname, isFullManifests, etag, cacheBytes); | ||
| logs.push(`[${isoNow()}] 🟢 Update Cache Success.`); | ||
| } | ||
| await this.taskService.finishTask(task, TaskState.Success, logs.join('\n')); | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The executeTask method in ProxyCacheService logs errors adequately, as shown by the repeated logging statements for failures. However, it lacks logging for the start and successful completion of tasks, which are important for comprehensive debugging and monitoring of task execution. This should be addressed to fully meet the requirements mentioned in the review comment.
Analysis chain
The methods createTask, findExecuteTask, and executeTask are well-implemented. Ensure that the task execution method logs all steps and errors adequately for debugging purposes.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify comprehensive logging in task execution methods
rg --type ts "executeTask" -A 50 | grep "logger"
Length of output: 7658
| this.logger.error(err); | ||
| } |
There was a problem hiding this comment.
Ensure error handling is robust. Consider adding more specific error information to help with debugging.
- this.logger.error(err);
+ this.logger.error('Error processing proxy cache update:', err);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| this.logger.error(err); | |
| } | |
| this.logger.error('Error processing proxy cache update:', err); | |
| } |
Add unit tests to cover the error handling in the subscribe method.
Would you like me to help by adding some test cases for this scenario?
| async subscribe() { | ||
| if (this.config.cnpmcore.syncMode !== SyncMode.proxy) return; | ||
| if (executingCount >= this.config.cnpmcore.syncPackageWorkerMaxConcurrentTasks) return; | ||
|
|
||
| executingCount++; | ||
| try { | ||
| let task = await this.proxyCacheService.findExecuteTask(); | ||
| while (task) { | ||
| const startTime = Date.now(); | ||
| this.logger.info('[SyncProxyCacheWorker:subscribe:executeTask:start][%s] taskId: %s, targetName: %s, attempts: %s, params: %j, updatedAt: %s, delay %sms', | ||
| executingCount, task.taskId, task.targetName, task.attempts, task.data, task.updatedAt, | ||
| startTime - task.updatedAt.getTime()); | ||
| await this.proxyCacheService.executeTask(task); | ||
| const use = Date.now() - startTime; | ||
| this.logger.info('[SyncProxyCacheWorker:subscribe:executeTask:success][%s] taskId: %s, targetName: %s, use %sms', | ||
| executingCount, task.taskId, task.targetName, use); | ||
| if (executingCount >= this.config.cnpmcore.syncPackageWorkerMaxConcurrentTasks) { | ||
| this.logger.info('[SyncProxyCacheWorker:subscribe:executeTask] current sync task count %s, exceed max concurrent tasks %s', | ||
| executingCount, this.config.cnpmcore.syncPackageWorkerMaxConcurrentTasks); | ||
| break; | ||
| } | ||
| // try next task | ||
| task = await this.proxyCacheService.findExecuteTask(); | ||
| } | ||
| } catch (err) { | ||
| this.logger.error('[SyncProxyCacheWorker:subscribe:executeTask:error][%s] %s', executingCount, err); | ||
| } finally { | ||
| executingCount--; | ||
| } |
There was a problem hiding this comment.
Ensure that the concurrency control with executingCount is thread-safe to avoid race conditions.
| task = await this.proxyCacheService.findExecuteTask(); | ||
| } | ||
| } catch (err) { | ||
| this.logger.error('[SyncProxyCacheWorker:subscribe:executeTask:error][%s] %s', executingCount, err); |
There was a problem hiding this comment.
Add unit tests to cover the error handling in the subscribe method.
Would you like me to help by adding some test cases for this scenario?
| async saveProxyCache(proxyCacheEntity: ProxyCacheEntity) { | ||
| let model = proxyCacheEntity.version ? | ||
| await this.ProxyCache.findOne({ fullname: proxyCacheEntity.fullname, version: proxyCacheEntity.version, fileType: proxyCacheEntity.fileType }) : | ||
| await this.ProxyCache.findOne({ fullname: proxyCacheEntity.fullname, fileType: proxyCacheEntity.fileType }); | ||
| if (model) { | ||
| model.updatedAt = proxyCacheEntity.updatedAt; | ||
| await model.save(); | ||
| } else { | ||
| try { | ||
| model = await ModelConvertor.convertEntityToModel(proxyCacheEntity, this.ProxyCache); | ||
| } catch (e) { | ||
| e.message = '[ProxyCacheRepository] insert ProxyCache failed: ' + e.message; | ||
| throw e; | ||
| } | ||
| } | ||
| return model; | ||
| } |
There was a problem hiding this comment.
Ensure that the saveProxyCache method's error handling is covered by unit tests, particularly the modification of the error message.
Would you like me to help by adding some test cases for this scenario?
| let { blockReason, manifest, pkg } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionSpec, isSync, isFullManifests); | ||
| const fileType = isFullManifests ? DIST_NAMES.MANIFEST : DIST_NAMES.ABBREVIATED; | ||
| if (!pkg) { | ||
| const allowSync = this.getAllowSync(ctx); | ||
| throw this.createPackageNotFoundErrorWithRedirect(fullname, undefined, allowSync); | ||
| if (this.config.cnpmcore.syncMode === SyncMode.proxy) { | ||
| try { | ||
| manifest = await this.proxyCacheService.getPackageVersionManifest(fullname, fileType, versionSpec); | ||
| } catch (error) { | ||
| // 缓存manifest错误，创建刷新缓存任务 | ||
| await this.proxyCacheService.createTask(`${fullname}/${fileType}`, { fullname, fileType }); | ||
| throw error; | ||
| } | ||
| } else { | ||
| const allowSync = this.getAllowSync(ctx); | ||
| throw this.createPackageNotFoundErrorWithRedirect(fullname, undefined, allowSync); | ||
| } | ||
| } | ||
| if (blockReason) { | ||
| this.setCDNHeaders(ctx); | ||
| throw this.createPackageBlockError(blockReason, fullname, versionSpec); | ||
| } | ||
| if (!manifest) { | ||
| throw new NotFoundError(`${fullname}@${versionSpec} not found`); | ||
| if (this.config.cnpmcore.syncMode === SyncMode.proxy) { | ||
| manifest = await this.proxyCacheService.getPackageVersionManifest(fullname, fileType, versionSpec); | ||
| } else { | ||
| throw new NotFoundError(`${fullname}@${versionSpec} not found`); | ||
| } |
There was a problem hiding this comment.
Add unit tests to cover the error handling and cache refresh task creation in the show method when in proxy mode.
Would you like me to help by adding some test cases for this scenario?
| if (this.config.cnpmcore.syncMode === SyncMode.proxy) { | ||
| // proxy mode | ||
| const fileType = isFullManifests ? DIST_NAMES.FULL_MANIFESTS : DIST_NAMES.ABBREVIATED_MANIFESTS; | ||
| let pkgManifest; | ||
| try { | ||
| pkgManifest = await this.proxyCacheService.getPackageManifest(fullname, fileType); | ||
| } catch (error) { | ||
| // 缓存manifest错误，创建刷新缓存任务 | ||
| await this.proxyCacheService.createTask(`${fullname}/${fileType}`, { fullname, fileType }); | ||
| throw error; | ||
| } | ||
| const nfsBytes = Buffer.from(JSON.stringify(pkgManifest)); | ||
| const { shasum: etag } = await calculateIntegrity(nfsBytes); | ||
| result = { data: pkgManifest, etag, blockReason: '' }; | ||
| } else { | ||
| result = await this.packageManagerService.listPackageAbbreviatedManifests(scope, name, isSync); | ||
| // sync mode | ||
| if (isFullManifests) { | ||
| result = await this.packageManagerService.listPackageFullManifests(scope, name, isSync); | ||
| } else { | ||
| result = await this.packageManagerService.listPackageAbbreviatedManifests(scope, name, isSync); | ||
| } |
There was a problem hiding this comment.
Add unit tests to cover the error handling and cache refresh task creation in the show method when in proxy mode.
Would you like me to help by adding some test cases for this scenario?
|
上游仓库可能会302返回一个重定向的地址，内网无法访问上游仓库返回的302地址。直接用egg-js的proxy插件不合适，这样的话proxy这个插件也有点不需要了 |
|
好像之前想的有点问题，代理模式下可以不用管本地数据库的内容。依赖的manifest能从proxyCache读到就用缓存的，没有就返回上游的流，因为代理模式下只要创建的任务是指定版本的，本地的永远是不完整的，不用和其他模式的逻辑在一起。 |
#366 开启代理模式时如果找不到依赖会直接返回上游仓库的manifest信息并缓存于nfs，当请求的tgz文件不存在时从上游仓库获取并返回，同时创建对应版本的同步任务。每小时检查更新已缓存的manifest文件保证上游仓库发布新版本时不会因为缓存落后而404。
Summary by CodeRabbit
ProxyCachefunctionality for caching and retrieving package manifests.SyncMode.proxyfor enhanced synchronization operations.