refactor(release-cleanup): 將清理邏輯由 bash 改寫為 Node.js #5

Closed
jiantw83 wants to merge 43 commits from refactor/nodejs-rewrite-20260626-103443 into develop
3 changed files with 26 additions and 20 deletions
Showing only changes of commit f6d2ebeab2 - Show all commits
+2 -18
View File
@@ -4,7 +4,7 @@ import { loadConfig } from './config.js'
import { GiteaClient } from './gitea-client.js'
import { cleanupReleases } from './releases.js'
import { cleanupOrphanTags } from './tags.js'
import { separator, fail } from './logger.js'
import { separator, failError } from './logger.js'
/**
* Action 主流程:讀取並驗證環境設定、建立帶認證的 Gitea 客戶端,
1
@@ -24,26 +24,10 @@ export async function main() {
separator()
}
Ghost marked this conversation as resolved
Review

嚴重等級🔴 嚴重
審查員:Mage
問題:當 main().catch(...) 捕捉到錯誤並執行 process.exit(1) 時,僅輸出 error.message。若錯誤是由 fetch 網路層拋出(例如 DNS 解析失敗或 connection refused),Node.js 的 Error 物件可能不包含足夠的上下文訊息,導致使用者難以區分是 API 回傳錯誤還是程式碼執行期錯誤。
建議:建議在 catch 區塊中,若錯誤是 Error 物件,輸出 error.stack 或至少記錄錯誤類型,並區分不同層級的例外(例如:驗證錯誤 vs. API 請求錯誤)。

**嚴重等級**:🔴 嚴重 **審查員**:Mage **問題**:當 `main().catch(...)` 捕捉到錯誤並執行 `process.exit(1)` 時,僅輸出 `error.message`。若錯誤是由 `fetch` 網路層拋出(例如 DNS 解析失敗或 connection refused),Node.js 的 `Error` 物件可能不包含足夠的上下文訊息,導致使用者難以區分是 API 回傳錯誤還是程式碼執行期錯誤。 **建議**:建議在 `catch` 區塊中,若錯誤是 `Error` 物件,輸出 `error.stack` 或至少記錄錯誤類型,並區分不同層級的例外(例如:驗證錯誤 vs. API 請求錯誤)。
/**
* 將錯誤輸出至 stderr,並盡量保留可供除錯的上下文(錯誤類型與堆疊),
* 以便區分設定驗證錯誤與網路/API 請求錯誤。
* @param {unknown} error 捕捉到的錯誤
*/
function reportFatal(error) {
if (error instanceof Error) {
fail(`${error.name}: ${error.message}`)
if (error.stack) {
process.stderr.write(`${error.stack}\n`)
}
} else {
fail(String(error))
}
}
// 僅在直接以 `node index.js` 執行時啟動主流程;被測試 import 時不自動執行,方便撰寫整合測試。
if (import.meta.url === `file://${process.argv[1]}`) {
main().catch((error) => {
reportFatal(error)
failError(error)
process.exit(1)
Ghost marked this conversation as resolved
Review

嚴重等級🔴 嚴重
審查員:Maya
問題:main 函式執行失敗時會觸發 process.exit(1),確保 CI/CD 流程能正確偵測錯誤。目前的測試僅驗證了 Promise 被 reject,但並未驗證程式是否真的正確以非零狀態碼結束。
建議:請在 app/test/main.test.js 中,模擬 main 拋出錯誤的情境,並透過 mock process.exit 來驗證當 main 執行失敗時,程式碼確實執行了 process.exit(1)。

**嚴重等級**:🔴 嚴重 **審查員**:Maya **問題**:main 函式執行失敗時會觸發 process.exit(1),確保 CI/CD 流程能正確偵測錯誤。目前的測試僅驗證了 Promise 被 reject,但並未驗證程式是否真的正確以非零狀態碼結束。 **建議**:請在 app/test/main.test.js 中,模擬 main 拋出錯誤的情境,並透過 mock process.exit 來驗證當 main 執行失敗時,程式碼確實執行了 process.exit(1)。
})
}
1
+17
View File
3
@@ -51,3 +51,20 @@ export function warn(message) {
export function fail(message) {
process.stderr.write(`[ERR] ${message}\n`)
}
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Assassin
問題:儘管使用了 stderr 輸出錯誤,但在 failError 中直接輸出 error.stack 可能會洩漏專案目錄結構、內部函式名稱等敏感路徑資訊。
建議:在生產環境下考慮隱藏堆疊追蹤,或僅在特定 debug 模式下輸出堆疊。

**嚴重等級**:🔵 建議 **審查員**:Assassin **問題**:儘管使用了 `stderr` 輸出錯誤,但在 `failError` 中直接輸出 `error.stack` 可能會洩漏專案目錄結構、內部函式名稱等敏感路徑資訊。 **建議**:在生產環境下考慮隱藏堆疊追蹤,或僅在特定 debug 模式下輸出堆疊。
/**
* 輸出錯誤至 stderr,並依錯誤型別保留可供除錯的上下文:
* 若為 Error 物件,輸出「名稱: 訊息」並附上堆疊;否則輸出其字串形式。
* 將「如何輸出錯誤」的邏輯集中於此,呼叫端毋須自行操作 stderr 或判斷型別。
* @param {unknown} error 待輸出的錯誤
*/
export function failError(error) {
if (error instanceof Error) {
fail(`${error.name}: ${error.message}`)
if (error.stack) {
process.stderr.write(`${error.stack}\n`)
}
} else {
fail(String(error))
}
}
+7 -2
View File
@@ -1,6 +1,11 @@
// 參數驗證,對應原本 entrypoint.sh 的 is_empty_or_null/require_value/require_integer。
// 驗證失敗時丟出 Error,由進入點統一捕捉後以非零狀態結束。
// 允許的 URL 協定;集中為常數,方便檢視系統允許的協定與日後擴充。
const ALLOWED_URL_PROTOCOLS = ['http:', 'https:']
// 合法 repository 形式:owner/repo,僅允許英數字與 . _ -,且恰好一個 /。
const REPOSITORY_PATTERN = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/
/**
* 判斷值是否視為「空」。使用嚴格相等,因此 `0`、`false`、字串 `"0"` 都不算空。
* @param {*} value 待判斷的值
3
@@ -49,7 +54,7 @@ export function requireUrl(name, value) {
} catch {
throw new Error(`${name} must be a valid URL`)
}
if (url.protocol !== 'http:' && url.protocol !== 'https:') {
if (!ALLOWED_URL_PROTOCOLS.includes(url.protocol)) {
throw new Error(`${name} must use http or https protocol`)
}
}
@@ -62,7 +67,7 @@ export function requireUrl(name, value) {
* @throws {Error} 格式不符或含 `..` 時丟出 `${name} must be in the form owner/repo without path traversal`
*/
export function requireRepository(name, value) {
if (String(value).includes('..') || !/^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/.test(value)) {
if (String(value).includes('..') || !REPOSITORY_PATTERN.test(value)) {
throw new Error(`${name} must be in the form owner/repo without path traversal`)
}
}