refactor: quality overhaul (#234)

* fix: prevent queue deadlock after cancelling tasks

Get no longer recurses while holding the mutex.
Cancelled queued tasks leave the map, making their IDs reusable.
Closed empty queues return ErrQueueClosed.

* fix: init task queue lazily and close on shutdown

AddTask is safe before Run by initializing the queue once.
Close unblocks workers waiting in Get.

* fix: make resource fingerprints deterministic

Sort map keys before hashing so Resource.ID is stable.

* fix: harden per-item processing tracking in tasks

Use the resource fingerprint as the dedup key on insert and delete.
Check and set processing entries atomically instead of TOCTOU.
Count only successful downloads.

* fix: honor IgnoreErrors in batch tasks

Element failures no longer cancel sibling elements or stop later groups.

* fix: report streamed upload bytes in batch tasks

Stream downloads report their byte count as the upload total.

* fix: validate i18n key parity across locales

geni18n fails when a language file misses any key.
Align the syncpeers completion key between en and zh-Hans.
Translate three untranslated parse keys in zh-Hans.

* refactor: share progress throttling helpers

Move size-tiered and count-based throttling into progressutil.
Localize hardcoded Chinese progress strings.
Drop five duplicated implementations and dead local copies.

* refactor: share unique filename logic

Storage backends use fsutil.UniquePath instead of local loops.

* fix: sanitize local storage paths

Reject absolute paths and dot-dot escapes in Save.
Check close errors and wrap creation failures.

* fix: preserve webdav error causes

Wrap mkdir and write failures with %w and drop dead error values.

* fix: kill rclone subprocess on reader close

Prevent cat processes from hanging after the pipe is closed.

* fix: fail alist init instead of exiting

Replace log.Fatalf with wrapped errors so a bad alist cannot kill the bot.
Cancel token refresh with the init context and re-login on 401.

* fix: enforce telegram album limits and track saved paths

Use tglimit.MaxAlbumItems for album batching and video splitting.
Exists now reports previously saved paths instead of always false.

* fix: guard storage registry maps

Protect Storages and UserStorages with mutexes and expose read accessors.

* fix: harden JS parser plugin runtime

Validate semver instead of panicking and require canHandle.
Recover plugin worker panics and time out CanHandle calls.
Return a copy from the registry and sanitize install filenames.

* fix: guard kemono parser against nil fields

Skip sparse preview and attachment entries instead of panicking.

* refactor: remove commented-out dead code

Drop unused kemono legacy types and commented response structs.

* test: cover invalid plugin version rejection

* fix: show all queued tasks in /task list

Render up to ten tasks and append the truncation note once.

* fix: guard callback data parsing

Reject malformed callback payloads before indexing split parts.

* fix: isolate media groups per user

Key pending groups by chat, user and group id.

* fix: require permission for callback handlers

* fix: initialize userbot context once

Replace the racy lazy init with sync.OnceValue.

* fix: avoid leaking raw errors in /dir reply

* fix: notify users on invalid update version

* fix: fail fast when API listen fails

Bind synchronously and surface errors instead of logging them.

* fix: add timeouts and backoff to webhook delivery

* refactor: remove dead code from api and bot

Drop the empty ProgressTracker shim, unused token context key and a redundant SetBotCommands call.

* fix: load remote config without local lookup

Skip the local file search after reading a config URL and add a timeout.

* refactor: drop unused hook config

* docs: document parser plugin config

* ci: fix BuildTime formatting and align checkout

Actions format does not format dates; pass the raw timestamp.

* chore: ignore cache directory

* fix: make cache init idempotent

* fix: upload only downloaded batch elements

Failed elements keep partial cache files and never reach the backend.
Successfully downloaded siblings still upload when one element fails.

* fix: record telegram saved paths only after upload

Skip-large returns a sentinel so skipped files are not marked as saved.

* fix: deduplicate alist token refreshes

Guard token access with a mutex and merge concurrent logins.
Reuse a recent refresh to avoid login storms.

* test: cover concurrent alist 401 retry

Ten parallel uploads share a single re-login under -race.

* fix: count parsed resources in progress text

* fix: localize storage lookup errors in /dir

Use the shared i18n key and escape the dynamic error.

* fix: deduplicate concurrent storage initialization

singleflight merges first-time inits so side effects are not duplicated.

* fix: guard nil progress trackers in api-created tasks

Batch, telegraph and transfer tasks run without a Telegram tracker
when created through the API; their callbacks must not panic.

* fix: keep album order when filtering failed batch items

Download results are stored by original index so surviving
elements keep their source order.

* test: cover nil-tracker task execution

* fix: report partial failure in batch done message

IgnoreErrors runs with failed elements show success and failed
counts instead of claiming every file completed.

* fix: skip login refresh for token-only alist storage

401 responses surface the auth error instead of sending a
credential-less login; the refresh window never exceeds TokenExp.

* test: cover alist refresh semantics

Concurrent refresh uses username/password; token-only storage
never attempts a login on 401.

* fix: call tracker from notifyProgress instead of recursing

The helper called itself, overflowing the stack on any batch
task with a progress tracker.

* fix: propagate cancellation past IgnoreErrors

Cancelled tasks must not be reported as successful: only
ordinary element failures are ignored.

* test: cover notifyProgress tracker call

* test: cover cancellation with IgnoreErrors

* fix: refresh alist token after startup 401s

The init login no longer satisfies the dedup window, so an
early 401 triggers a real refresh; later 401s reuse it.
Clarify that streaming uploads cannot replay their body.

* test: exercise the alist 401 refresh path

Concurrent uploads now reject the init token, share one
refresh and retry with the new token; token-only stays inert.

* style: trim verbose comments

Drop process-style explanations; keep one-line behavior notes.

* style: gofmt test files

* fix: drop unbounded saved-path cache in telegram storage

Telegram cannot reliably query remote file existence, so the
cache answered a question it could not answer and grew without
bound. Exists returns false again, as before.

* style: format codes
This commit is contained in:
Krau
2026-08-17 19:10:03 +08:00
committed by GitHub
parent 6fd95bbe1b
commit c2f8ab3c01
79 changed files with 1276 additions and 764 deletions
+7 -1
View File
@@ -44,7 +44,10 @@ func jsRegisterParser(vm *goja.Runtime) func(call goja.FunctionCall) goja.Value
return vm.NewGoError(errors.New("metadata cannot be null or undefined"))
}
pluginV := semver.MustParse(metadata.Version)
pluginV, err := semver.Parse(metadata.Version)
if err != nil {
return vm.NewGoError(fmt.Errorf("invalid parser version %q: %w", metadata.Version, err))
}
if pluginV.LT(MinimumParserVersion) {
return vm.NewGoError(fmt.Errorf("parser version %s is not supported, must be at least %s", metadata.Version, MinimumParserVersion))
}
@@ -57,6 +60,9 @@ func jsRegisterParser(vm *goja.Runtime) func(call goja.FunctionCall) goja.Value
if parseFn == nil || goja.IsUndefined(parseFn) {
return vm.NewGoError(errors.New("parser must provide a parse function"))
}
if handleFn == nil || goja.IsUndefined(handleFn) {
return vm.NewGoError(errors.New("parser must provide a canHandle function"))
}
parsers.Add(newJSParser(vm, handleFn, parseFn, metadata))
return goja.Undefined()
}
+71 -36
View File
@@ -9,6 +9,7 @@ import (
"os"
"path/filepath"
"sync"
"time"
"github.com/charmbracelet/log"
"github.com/dop251/goja"
@@ -34,11 +35,23 @@ type jsParserResp struct {
err error
}
const canHandleTimeout = 10 * time.Second
func (p *jsParser) CanHandle(url string) bool {
respCh := make(chan jsParserResp, 1)
p.reqCh <- jsParserReq{method: ParserMethodCanHandle, url: url, respCh: respCh}
resp := <-respCh
return resp.ok && resp.err == nil
timer := time.NewTimer(canHandleTimeout)
defer timer.Stop()
select {
case p.reqCh <- jsParserReq{method: ParserMethodCanHandle, url: url, respCh: respCh}:
case <-timer.C:
return false
}
select {
case resp := <-respCh:
return resp.ok && resp.err == nil
case <-timer.C:
return false
}
}
func (p *jsParser) Parse(ctx context.Context, url string) (*parser.Item, error) {
@@ -61,41 +74,57 @@ func newJSParser(vm *goja.Runtime, canHandleFunc, parseFunc goja.Value, metadata
go func() {
for req := range p.reqCh {
switch req.method {
case ParserMethodCanHandle:
fn, _ := goja.AssertFunction(canHandleFunc)
res, err := fn(goja.Undefined(), p.vm.ToValue(req.url))
if err != nil {
req.respCh <- jsParserResp{ok: false, err: err}
continue
}
req.respCh <- jsParserResp{ok: res.ToBoolean()}
case ParserMethodParse:
fn, _ := goja.AssertFunction(parseFunc)
result, err := fn(goja.Undefined(), p.vm.ToValue(req.url))
if err != nil {
req.respCh <- jsParserResp{err: err}
continue
}
var item parser.Item
if exported := result.Export(); exported != nil {
data, err := json.Marshal(exported)
func() {
defer func() {
if r := recover(); r != nil {
log.Errorf("JS parser %q panicked while handling method %d: %v", p.meta.Name, req.method, r)
req.respCh <- jsParserResp{err: fmt.Errorf("JS parser %q panicked: %v", p.meta.Name, r)}
}
}()
switch req.method {
case ParserMethodCanHandle:
fn, ok := goja.AssertFunction(canHandleFunc)
if !ok {
req.respCh <- jsParserResp{err: fmt.Errorf("canHandle is not a function")}
return
}
res, err := fn(goja.Undefined(), p.vm.ToValue(req.url))
if err != nil {
req.respCh <- jsParserResp{err: fmt.Errorf("failed to marshal result to JSON: %w", err)}
continue
req.respCh <- jsParserResp{ok: false, err: err}
return
}
req.respCh <- jsParserResp{ok: res.ToBoolean()}
case ParserMethodParse:
fn, ok := goja.AssertFunction(parseFunc)
if !ok {
req.respCh <- jsParserResp{err: fmt.Errorf("parse is not a function")}
return
}
result, err := fn(goja.Undefined(), p.vm.ToValue(req.url))
if err != nil {
req.respCh <- jsParserResp{err: err}
return
}
if err := json.Unmarshal(data, &item); err != nil {
req.respCh <- jsParserResp{err: fmt.Errorf("failed to unmarshal JSON to Item: %w", err)}
continue
var item parser.Item
if exported := result.Export(); exported != nil {
data, err := json.Marshal(exported)
if err != nil {
req.respCh <- jsParserResp{err: fmt.Errorf("failed to marshal result to JSON: %w", err)}
return
}
if err := json.Unmarshal(data, &item); err != nil {
req.respCh <- jsParserResp{err: fmt.Errorf("failed to unmarshal JSON to Item: %w", err)}
return
}
} else {
req.respCh <- jsParserResp{err: fmt.Errorf("JS function returned null or undefined")}
return
}
} else {
req.respCh <- jsParserResp{err: fmt.Errorf("JS function returned null or undefined")}
continue
req.respCh <- jsParserResp{item: &item}
}
req.respCh <- jsParserResp{item: &item}
}
}()
}
}()
@@ -116,7 +145,8 @@ func LoadPlugins(ctx context.Context, dir string) error {
scriptPath := filepath.Join(dir, e.Name())
code, err := os.ReadFile(scriptPath)
if err != nil {
return err
log.Warnf("Failed to read plugin file %s: %v", e.Name(), err)
continue
}
vm := goja.New()
@@ -130,7 +160,8 @@ func LoadPlugins(ctx context.Context, dir string) error {
vm.Set("playwright", jsPlaywright(vm, logger))
if _, err := vm.RunString(string(code)); err != nil {
return fmt.Errorf("error loading plugin %s: %w", e.Name(), err)
logger.Warnf("Failed to load plugin %s: %v", e.Name(), err)
continue
}
}
return nil
@@ -164,8 +195,12 @@ func addPlugin(ctx context.Context, code string, name string) error {
if len(configuredDirs) > 0 {
dir = configuredDirs[0]
}
fileName := filepath.Base(name)
if fileName == "" || fileName == "." || fileName == ".." {
return fmt.Errorf("invalid plugin name %q", name)
}
if err := os.MkdirAll(dir, 0755); err == nil {
pluginPath := filepath.Join(dir, name)
pluginPath := filepath.Join(dir, fileName)
if err := os.WriteFile(pluginPath, []byte(code), 0644); err != nil {
logger.Warn("Failed to save plugin file: " + err.Error())
}
+35
View File
@@ -0,0 +1,35 @@
package js_test
import (
"os"
"path/filepath"
"testing"
"github.com/krau/SaveAny-Bot/parsers/js"
)
// Regression: a plugin with an invalid semver version must be rejected
// without panicking the process (previously semver.MustParse crashed the bot).
func TestLoadPluginsRejectsInvalidVersion(t *testing.T) {
dir := t.TempDir()
bad := `registerParser({
metadata: { name: "probe", version: "not-a-version", description: "", author: "" },
canHandle: function(url) { return true; },
parse: async function(url) { return { resources: [] }; }
});`
if err := os.WriteFile(filepath.Join(dir, "bad.js"), []byte(bad), 0o644); err != nil {
t.Fatal(err)
}
good := `registerParser({
metadata: { name: "good", version: "1.0.0", description: "", author: "" },
canHandle: function(url) { return false; },
parse: async function(url) { return { resources: [] }; }
});`
if err := os.WriteFile(filepath.Join(dir, "good.js"), []byte(good), 0o644); err != nil {
t.Fatal(err)
}
// Must not panic; the bad plugin is skipped and remaining plugins load.
if err := js.LoadPlugins(t.Context(), dir); err != nil {
t.Fatalf("LoadPlugins returned error: %v", err)
}
}
+9
View File
@@ -10,6 +10,7 @@ import (
"path"
"strings"
"github.com/charmbracelet/log"
"github.com/duke-git/lancet/v2/strutil"
"github.com/krau/SaveAny-Bot/common/utils/netutil"
"github.com/krau/SaveAny-Bot/pkg/parser"
@@ -135,9 +136,17 @@ func (k *KemonoParser) parseOne(ctx context.Context, info *DownloadInfo) (*parse
if preview.Type == nil || *preview.Type != "thumbnail" {
continue
}
if preview.Path == nil || preview.Server == nil {
log.FromContext(ctx).Warnf("Skipping kemono preview with missing path or server: post %s", info.PostID)
continue
}
picCdnMap[*preview.Path] = *preview.Server
}
for _, attachment := range postInfo.Post.Attachments {
if attachment.Path == nil || attachment.Name == nil {
log.FromContext(ctx).Warnf("Skipping kemono post attachment with missing path or name: post %s", info.PostID)
continue
}
if !isImageExt(*attachment.Path) {
continue
}
-16
View File
@@ -1,16 +0,0 @@
package kemono
type PostLegacy struct {
Props Props `json:"props"`
Results []Result `json:"results"`
}
type Props struct {
Count uint `json:"count"`
Limit uint `json:"limit"`
}
type Result struct {
ID string `json:"id"`
Title string `json:"title"`
}
-8
View File
@@ -1,8 +0,0 @@
package kemono
type UserProfile struct {
ID string `json:"id"`
Name string `json:"name"`
Service string `json:"service"`
PublicID *string `json:"public_id,omitempty"`
}
-96
View File
@@ -1,101 +1,5 @@
package twitter
// type AutoGenerated struct {
// Code int `json:"code"`
// Message string `json:"message"`
// Tweet struct {
// URL string `json:"url"`
// ID string `json:"id"`
// Text string `json:"text"`
// RawText struct {
// Text string `json:"text"`
// Facets []struct {
// Type string `json:"type"`
// Indices []int `json:"indices"`
// Original string `json:"original"`
// ID string `json:"id,omitempty"`
// Display string `json:"display,omitempty"`
// Replacement string `json:"replacement,omitempty"`
// } `json:"facets"`
// } `json:"raw_text"`
// Author struct {
// ID string `json:"id"`
// Name string `json:"name"`
// ScreenName string `json:"screen_name"`
// AvatarURL string `json:"avatar_url"`
// BannerURL interface{} `json:"banner_url"`
// Description string `json:"description"`
// Location string `json:"location"`
// URL string `json:"url"`
// Followers int `json:"followers"`
// Following int `json:"following"`
// Joined string `json:"joined"`
// Likes int `json:"likes"`
// MediaCount int `json:"media_count"`
// Protected bool `json:"protected"`
// Website struct {
// URL string `json:"url"`
// DisplayURL string `json:"display_url"`
// } `json:"website"`
// Tweets int `json:"tweets"`
// AvatarColor interface{} `json:"avatar_color"`
// } `json:"author"`
// Replies int `json:"replies"`
// Retweets int `json:"retweets"`
// Likes int `json:"likes"`
// Bookmarks int `json:"bookmarks"`
// CreatedAt string `json:"created_at"`
// CreatedTimestamp int `json:"created_timestamp"`
// PossiblySensitive bool `json:"possibly_sensitive"`
// Views int `json:"views"`
// IsNoteTweet bool `json:"is_note_tweet"`
// CommunityNote interface{} `json:"community_note"`
// Lang string `json:"lang"`
// ReplyingTo interface{} `json:"replying_to"`
// ReplyingToStatus interface{} `json:"replying_to_status"`
// Media struct {
// All []struct {
// URL string `json:"url"`
// ThumbnailURL string `json:"thumbnail_url"`
// Duration int `json:"duration"`
// Width int `json:"width"`
// Height int `json:"height"`
// Format string `json:"format"`
// Type string `json:"type"`
// Variants []struct {
// Bitrate int `json:"bitrate"`
// ContentType string `json:"content_type"`
// URL string `json:"url"`
// } `json:"variants"`
// } `json:"all"`
// Photos []struct {
// Type string `json:"type"`
// URL string `json:"url"`
// Width int `json:"width"`
// Height int `json:"height"`
// } `json:"photos"`
// Videos []struct {
// URL string `json:"url"`
// ThumbnailURL string `json:"thumbnail_url"`
// Duration int `json:"duration"`
// Width int `json:"width"`
// Height int `json:"height"`
// Format string `json:"format"`
// Type string `json:"type"`
// Variants []struct {
// Bitrate int `json:"bitrate"`
// ContentType string `json:"content_type"`
// URL string `json:"url"`
// } `json:"variants"`
// } `json:"videos"`
// } `json:"media"`
// Source string `json:"source"`
// TwitterCard string `json:"twitter_card"`
// Color interface{} `json:"color"`
// Provider string `json:"provider"`
// } `json:"tweet"`
// }
type FxTwitterApiResp struct {
Code int `json:"code"`
Message string `json:"message"`
+2 -1
View File
@@ -2,6 +2,7 @@ package parsers
import (
"fmt"
"slices"
"sync"
"github.com/krau/SaveAny-Bot/config"
@@ -39,5 +40,5 @@ func Get() []parser.Parser {
configOnce.Do(configParsers)
mu.Lock()
defer mu.Unlock()
return parsers
return slices.Clone(parsers)
}