Ci/discord release announce - #539
Conversation
Optimization(zh-Hant): 發布新版本時自動發到 Discord Optimization(en-US): a new release is announced on Discord automatically The step lives in the release job rather than in a workflow listening on `release`, because GitHub refuses to start a run from an event its own GITHUB_TOKEN caused and `gh release create` uses exactly that token — an `on: release` workflow would sit silent forever and look like a Discord fault. Listening for both `published` and `prereleased` would also have announced every snapshot twice. A note is written for GitHub and does not fit Discord: 37,724 characters for 26w34c against a 4,096-character embed. The English block goes because it lives in `<details>`, which Discord does not render; the platform badges go because an embed renders no images and they would arrive as literal markdown; and the commit URLs go because keeping them costs 8,062 characters and the release page is linked at the bottom. When it still overflows, the short hashes go before any entry does — losing a hash costs a click, losing an entry means a change shipped and nobody was told. What is left is split evenly between the three categories, each saying how much it is holding back, because filling from the top spends the whole budget on 新功能 and posts 錯誤修正 as an empty heading. Announcement failure is a warning, not a red job: by then the build is published and the artifacts are downloadable, and failing the run would say the release did not happen.
A custom emoji's id is visible to everyone who can see the server, and useless without the webhook that is not — so it was a secret for no reason, and one more thing to configure before the announcement worked at all. In the workflow rather than as defaults in the script: a fork announcing to its own server then falls back to the unicode pair instead of referencing emoji it does not have.
|
🔍 OpenCodeReview found 5 issue(s) in this PR.
|
| python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" || | ||
| echo "::warning::Discord announcement failed — the release itself is fine" |
There was a problem hiding this comment.
建議將 ${{ steps.publish.outputs.tag }} 透過環境變數傳遞,而非直接在 run 腳本中使用 GitHub Actions 表達式。這可以避免潛在的指令注入(Script Injection)風險。雖然 release 的 tag 通常是可信的,但為了符合安全最佳實踐,建議將所有外部輸入(包含 step outputs)透過 env 區塊傳遞,以確保 shell 執行時將其視為純字串。
Suggestion:
| python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" || | |
| echo "::warning::Discord announcement failed — the release itself is fine" | |
| env: | |
| DISCORD_WEBHOOK: ${{ secrets.DISCORD_WEBHOOK }} | |
| DPIP_EMOJI_ANDROID: "<:android:1539201870935892028>" | |
| DPIP_EMOJI_IOS: "<:ios:1539201897766981692>" | |
| GITHUB_TOKEN: ${{ github.token }} | |
| TAG: ${{ steps.publish.outputs.tag }} | |
| run: | | |
| if [ -z "${DISCORD_WEBHOOK:-}" ]; then | |
| echo "::notice::DISCORD_WEBHOOK is not set — nothing announced" | |
| exit 0 | |
| fi | |
| python3 tool/release/discord.py "$TAG" || | |
| echo "::warning::Discord announcement failed — the release itself is fine" |
| python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" || | ||
| echo "::warning::Discord announcement failed — the release itself is fine" |
There was a problem hiding this comment.
建議將 ${{ steps.publish.outputs.tag }} 透過環境變數傳遞,而非直接在 run 腳本中使用 GitHub Actions 表達式。這可以避免潛在的指令注入(Script Injection)風險。雖然 release 的 tag 通常是可信的,但為了符合安全最佳實踐,建議將所有外部輸入(包含 step outputs)透過 env 區塊傳遞,以確保 shell 執行時將其視為純字串。
Suggestion:
| python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" || | |
| echo "::warning::Discord announcement failed — the release itself is fine" | |
| env: | |
| DISCORD_WEBHOOK: ${{ secrets.DISCORD_WEBHOOK }} | |
| DPIP_EMOJI_ANDROID: "<:android:1539201870935892028>" | |
| DPIP_EMOJI_IOS: "<:ios:1539201897766981692>" | |
| GITHUB_TOKEN: ${{ github.token }} | |
| TAG: ${{ steps.publish.outputs.tag }} | |
| run: | | |
| if [ -z "${DISCORD_WEBHOOK:-}" ]; then | |
| echo "::notice::DISCORD_WEBHOOK is not set — nothing announced" | |
| exit 0 | |
| fi | |
| python3 tool/release/discord.py "$TAG" || | |
| echo "::warning::Discord announcement failed — the release itself is fine" |
| for attempt in range(3): | ||
| try: | ||
| return _get(path) | ||
| except urllib.error.HTTPError as error: | ||
| if error.code != 404 or attempt == 2: | ||
| raise | ||
| time.sleep(2 * (attempt + 1)) | ||
| raise AssertionError("unreachable") |
| def fit(text: str, room: int) -> str: | ||
| """The per-embed backstop: [share] bounds the message, this bounds one embed. | ||
|
|
||
| Cut on an item boundary. A description cut at the character limit ends | ||
| inside somebody's changelog entry, which reads as a bug in that entry | ||
| rather than as a limit in Discord. | ||
| """ | ||
| if len(text) <= room: | ||
| return text | ||
| cut = text.rfind("\n", 0, room - 20) | ||
| return text[: cut if cut > 0 else room - 20].rstrip() + "\n…" |
There was a problem hiding this comment.
在 fit 函數中,若 room 的值小於 20,room - 20 會為負數。在 rfind(sub, start, end) 中,當 end 為負數時,它代表從字串末尾開始的偏移量。這會導致搜尋範圍出錯,且若 cut 為正數,回傳的字串長度可能依然遠超 room 的限制,違反了截斷字串的初衷。建議加入 max(0, room - 20) 或對 room 的大小進行檢查。
Suggestion:
| def fit(text: str, room: int) -> str: | |
| """The per-embed backstop: [share] bounds the message, this bounds one embed. | |
| Cut on an item boundary. A description cut at the character limit ends | |
| inside somebody's changelog entry, which reads as a bug in that entry | |
| rather than as a limit in Discord. | |
| """ | |
| if len(text) <= room: | |
| return text | |
| cut = text.rfind("\n", 0, room - 20) | |
| return text[: cut if cut > 0 else room - 20].rstrip() + "\n…" | |
| def fit(text: str, room: int) -> str: | |
| if len(text) <= room: | |
| return text | |
| search_end = max(0, room - 20) | |
| cut = text.rfind("\n", 0, search_end) | |
| return text[: cut if cut > 0 else search_end].rstrip() + "\n…" |
| try: | ||
| with urllib.request.urlopen(request, timeout=20) as response: | ||
| print(f"discord: HTTP {response.status}") | ||
| return 0 | ||
| except urllib.error.HTTPError as error: | ||
| # Discord says what it disliked in the body; the exception alone says | ||
| # only the number, and a stack trace here names urllib rather than the | ||
| # field it rejected. | ||
| print(f"discord: HTTP {error.code}", file=sys.stderr) | ||
| print(error.read().decode(errors="replace")[:800], file=sys.stderr) | ||
| return 1 |
There was a problem hiding this comment.
api 函數與 main 函數中的 urllib.request.urlopen 呼叫僅捕捉了 urllib.error.HTTPError。當發生網路連線失敗、DNS 解析問題或連線超時等非 HTTP 錯誤時,會拋出 urllib.error.URLError,這會導致腳本直接崩潰並顯示 Traceback。建議一併捕捉 urllib.error.URLError 以提供更穩定的錯誤處理。
Suggestion:
| try: | |
| with urllib.request.urlopen(request, timeout=20) as response: | |
| print(f"discord: HTTP {response.status}") | |
| return 0 | |
| except urllib.error.HTTPError as error: | |
| # Discord says what it disliked in the body; the exception alone says | |
| # only the number, and a stack trace here names urllib rather than the | |
| # field it rejected. | |
| print(f"discord: HTTP {error.code}", file=sys.stderr) | |
| print(error.read().decode(errors="replace")[:800], file=sys.stderr) | |
| return 1 | |
| try: | |
| with urllib.request.urlopen(request, timeout=20) as response: | |
| print(f"discord: HTTP {response.status}") | |
| return 0 | |
| except (urllib.error.HTTPError, urllib.error.URLError) as error: | |
| if isinstance(error, urllib.error.HTTPError): | |
| print(f"discord: HTTP {error.code}", file=sys.stderr) | |
| print(error.read().decode(errors="replace")[:800], file=sys.stderr) | |
| else: | |
| print(f"discord: Network error: {error}", file=sys.stderr) | |
| return 1 |
這個 PR 做了什麼
相關 issue
怎麼驗
檢查清單
tool/check/commits.sh origin/main..HEAD通過—— commit 訊息就是更新日誌,格式見 commit.md
mise exec -- flutter analyze與mise exec -- flutter test通過AppLocalizations,沒有寫死AppSpacing/AppRadius/AppMotion,深色模式看過,文字對比度可接受