新增Suno官方Api支持 - #2354
Conversation
…-embedding-token-count fix: gemini batch embedding token not counted
fix: handle JSON parsing for thinking content in ollama stream
…-emotion 豆包语音2.0音色支持情感,情绪,音量
Comment out the debug log for MiniMax TTS Request.
增加MiniMax语音合成TTS支持
…ort-stream-options Ali channel support stream options
…ix-gemini-ImageConfig Revert "fix: gemini image correct generationConfig"
…emini-veo3.1-i2v Revert "Gemini Veo3.1[AI Studio]增加图生视频支持"
…emini-image-edit Revert "Gemini Image系列支持图像编辑"
…ix-nano-banana-err Revert "fix: nano-banana not compatible imageSize"
…dd-gemini-3-pro-image-preview-oai Revert "OAI生图接口支持gemini 3 pro image preview"
…dels (Midjourney, Rerank, Suno). Add OpenAPI specifications for backend management and relay interfaces.
…i-turn feat(gemini): implement markdown image handling in text processing
chore: update openapi files
WalkthroughRegisters the official Suno API URL, adds official vs non-official request/response handling for Suno tasks, extends DTOs and Task model with URL and Suno-specific fields, routes /api/v1/generate, and updates frontend to preview music URLs. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Router
participant Controller
participant Service
participant Adaptor
participant OfficialSunoAPI as "Suno (Official)"
participant Provider as "Non-official Provider"
participant DB
Client->>Router: POST /suno/api/v1/generate
Router->>Controller: RelayTask(ctx)
Controller->>Service: GetTaskAction(ctx)
Service-->>Controller: action
Controller->>Adaptor: FetchTask(baseURL, key, body)
Adaptor->>Adaptor: IsOfficialUrl(baseURL)?
alt official
Adaptor->>OfficialSunoAPI: GET (Authorization, callback)
OfficialSunoAPI-->>Adaptor: OfficialSunoResponse
Adaptor->>Adaptor: DoResponseOfficial -> ParseResponseItems
Adaptor->>DB: Save standardized TaskResponse (includes Url, times)
Adaptor-->>Controller: Standardized TaskResponse
else non-official
Adaptor->>Provider: POST (provider-specific)
Provider-->>Adaptor: Response
Adaptor->>Adaptor: json.Unmarshal / convert (ms→s)
Adaptor->>DB: Save/Update Task
Adaptor-->>Controller: Standardized TaskResponse
end
Controller-->>Client: HTTP response (task record with `url` when present)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
relay/channel/task/suno/adaptor.go (1)
163-168: Inconsistent resource cleanup:defer req.Body.Close()on a request with nil body.At line 168,
defer req.Body.Close()is called, but this is a POST request created withbytes.NewBuffer(byteBody)which is not the same asreq.Body. Thereq.Bodyshould not be closed by the caller for outgoing requests. This is dead/incorrect code that should be removed.req, err := http.NewRequest("POST", requestUrl, bytes.NewBuffer(byteBody)) if err != nil { common.SysLog(fmt.Sprintf("Get Task error: %v", err)) return nil, err } -defer req.Body.Close()controller/task.go (1)
227-235: Sorting JSON bytes is not a reliable comparison method.Sorting the individual bytes of serialized JSON does not produce meaningful results for comparing JSON objects. Two equivalent JSON objects could have different byte representations, and sorting bytes destroys structural meaning. Consider using
reflect.DeepEqualor comparing the unmarshaled structures instead.- oldData, _ := json.Marshal(oldTask.Data) - newData, _ := json.Marshal(newTask.Data) - - sort.Slice(oldData, func(i, j int) bool { - return oldData[i] < oldData[j] - }) - sort.Slice(newData, func(i, j int) bool { - return newData[i] < newData[j] - }) - - if string(oldData) != string(newData) { + if !bytes.Equal(oldTask.Data, newTask.Data) { return true }Note: If
Dataisjson.RawMessage, direct byte comparison without sorting is simpler and more correct for detecting changes. If semantic equality is needed, unmarshal and compare the structures.
🧹 Nitpick comments (5)
relay/constant/relay_mode.go (1)
136-144: Suno official path mapping looks correct; consider handling trailing slash as wellThe added
strings.HasSuffix(path, "/api/v1/generate")correctly classifies the official Suno submit endpoint (e.g./suno/api/v1/generate) asRelayModeSunoSubmit. This aligns with the documented official API.To make the routing a bit more robust to minor client variations, you might also want to treat a trailing slash as equivalent, e.g.:
- } else if strings.Contains(path, "/submit/") || strings.HasSuffix(path, "/api/v1/generate") { + } else if strings.Contains(path, "/submit/") || + strings.HasSuffix(path, "/api/v1/generate") || + strings.HasSuffix(path, "/api/v1/generate/") {Not mandatory, but it can prevent subtle 404/unknown-mode issues if someone sends
/suno/api/v1/generate/by mistake.relay/channel/task/suno/adaptor.go (1)
72-77: OnlySunoActionMusicis handled in the official URL switch.The switch statement only handles
SunoActionMusic. If other actions (likeSunoActionLyrics) are used with an official URL, they will fall through and use the default URL pattern/suno/submit/{action}, which may not be valid for the official API.Consider adding a default case to handle unsupported actions for official URLs:
if constant.IsOfficialUrl(baseURL) { switch info.Action { case constant.SunoActionMusic: fullRequestURL = fmt.Sprintf("%s/api/v1/generate", baseURL) + default: + return "", fmt.Errorf("action %s not supported for official Suno API", info.Action) } }controller/task.go (1)
204-221: Duplicate condition check incheckTaskNeedUpdate.Lines 210-211 check
oldTask.FinishTime != newTask.FinishTime, and lines 219-220 perform the exact same check again. This is redundant.Remove the duplicate check:
if oldTask.FinishTime != newTask.FinishTime { return true } - - if (oldTask.Status == model.TaskStatusFailure || oldTask.Status == model.TaskStatusSuccess) && oldTask.Progress != "100%" { - return true - } + if (oldTask.Status == model.TaskStatusFailure || oldTask.Status == model.TaskStatusSuccess) && oldTask.Progress != "100%" { + return true + } - - if oldTask.FinishTime != newTask.FinishTime { - return true - }relay/channel/task/suno/suno_official.go (2)
68-75:finishTimeuses current time instead of actual finish time from response.When
Status == "SUCCESS",finishTimeis set totime.Now().UnixMilli(). This captures the polling time, not the actual task completion time. If the official API provides a finish timestamp, that should be used instead.If the official API doesn't provide a finish time, consider documenting this limitation. Otherwise, extract it from the response data (e.g., from
OfficialSunoDataif available).
83-92:Actionfield is not populated inSunoDataResponse.The
dto.SunoDataResponsestruct has anActionfield (per the external snippet), butToStandardResponsedoesn't set it. This could cause issues if downstream code relies on the action type.Consider populating the
Actionfield:sunoDataResponse := dto.SunoDataResponse{ TaskID: r.Data.TaskID, + Action: r.Data.Type, // or appropriate mapping Status: r.Data.Status, FailReason: failReason,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
constant/channel.go(2 hunks)controller/task.go(5 hunks)dto/suno.go(2 hunks)middleware/distributor.go(1 hunks)model/task.go(1 hunks)relay/channel/task/suno/adaptor.go(6 hunks)relay/channel/task/suno/suno_official.go(1 hunks)relay/constant/relay_mode.go(1 hunks)router/relay-router.go(1 hunks)service/task.go(1 hunks)web/src/components/table/task-logs/TaskLogsColumnDefs.jsx(3 hunks)web/src/constants/common.constant.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.
Applied to files:
model/task.go
🧬 Code graph analysis (7)
service/task.go (1)
constant/task.go (2)
TaskPlatform(3-3)SunoActionMusic(11-11)
relay/channel/task/suno/adaptor.go (7)
service/task.go (1)
GetTaskAction(18-24)dto/suno.go (1)
SunoSubmitReq(11-25)common/gin.go (1)
UnmarshalBodyReusable(35-60)service/error.go (1)
TaskErrorWrapperLocal(134-138)setting/system_setting/system_setting_old.go (1)
ServerAddress(3-3)constant/channel.go (1)
IsOfficialUrl(183-190)constant/task.go (1)
SunoActionMusic(11-11)
web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (3)
web/src/constants/common.constant.js (2)
TASK_ACTION_MUSIC(45-45)TASK_ACTION_MUSIC(45-45)web/src/components/common/DocumentRenderer/index.jsx (1)
isUrl(32-39)web/src/hooks/task-logs/useTaskLogsData.js (1)
openVideoModal(265-268)
controller/task.go (4)
dto/suno.go (2)
TaskResponse(76-80)SunoDataResponse(31-41)relay/channel/task/suno/suno_official.go (1)
ParseResponseItems(166-178)common/json.go (1)
Unmarshal(9-11)common/constants.go (1)
StartTime(12-12)
router/relay-router.go (1)
controller/relay.go (1)
RelayTask(392-435)
middleware/distributor.go (2)
service/task.go (1)
GetTaskModelName(14-17)constant/task.go (1)
TaskPlatformSuno(6-6)
relay/channel/task/suno/suno_official.go (6)
types/error.go (1)
ErrorCode(36-36)dto/suno.go (2)
TaskResponse(76-80)SunoDataResponse(31-41)common/json.go (2)
Marshal(21-23)Unmarshal(9-11)service/error.go (1)
TaskErrorWrapper(140-157)common/sys_log.go (1)
SysLog(11-14)service/http_client.go (1)
GetHttpClient(49-51)
🔇 Additional comments (15)
router/relay-router.go (1)
170-171: LGTM! New route integrates cleanly.The new
/api/v1/generateendpoint is correctly added to the Suno router group with appropriate middleware and uses the existingRelayTaskhandler, maintaining consistency with other Suno routes.middleware/distributor.go (1)
179-179: LGTM! Refactoring centralizes model name resolution.The change from directly calling
CoverTaskActionToModelNamewithc.Param("action")to usingservice.GetTaskModelName(c, constant.TaskPlatformSuno)centralizes the action retrieval logic. This properly supports the new/api/v1/generateendpoint where the action is derived from the path rather than a URL parameter.constant/channel.go (2)
98-98: LGTM! Official Suno API URL added.The addition of the official Suno API base URL at index 36 correctly corresponds to
ChannelTypeSunoAPI = 36(line 36), maintaining consistency in the channel type mapping.
183-190: LGTM! Helper function for official URL detection.The
IsOfficialUrlfunction provides a simple way to verify if a base URL is in the official channel list, supporting the official Suno API integration.service/task.go (1)
14-24: LGTM! Helper functions centralize task action resolution.The new
GetTaskModelNameandGetTaskActionfunctions properly centralize the logic for deriving task actions and model names. The default behavior for the/api/v1/generateendpoint (defaulting to music action) aligns well with the PR objectives for official Suno API support.web/src/constants/common.constant.js (1)
45-45: LGTM! Constant added for music task identification.The new
TASK_ACTION_MUSICconstant enables the frontend to identify music generation tasks, aligning with the backend's Suno action handling.dto/suno.go (2)
21-24: LGTM! New fields support official Suno API.The added fields (
Instrumental,Model,CallBackUrl,CustomMode) properly support the official Suno API request parameters as described in the PR objectives, with appropriate JSON tags.
36-36: LGTM! Url field added with proper database mapping.The
Urlfield correctly includes thegorm:"type:text"tag for database persistence, appropriate for storing URLs which can be lengthy.web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (3)
42-42: LGTM! Import added for music task support.The import of
TASK_ACTION_MUSICconstant enables music task identification in the UI rendering logic.
364-369: LGTM! Improved URL resolution logic.The updated logic correctly prioritizes
record.url(from the new Task.Url field) and falls back torecord.fail_reason. The URL validation using a simple regex pattern is adequate for this use case.
386-398: LGTM! Music preview rendering implemented correctly.The new rendering branch for successful music tasks mirrors the video task pattern and appropriately reuses the existing
openVideoModalfor previewing music URLs. The link text "点击预览音乐" clearly indicates the preview functionality.model/task.go (1)
55-55: Verify database persistence for the Url field.The new
Urlfield has ajson:"url"tag but nogormtag. Verify whether this field should be persisted to the database. If persistence is intended, add agorm:"type:text"tag (similar to theUrlfield indto.SunoDataResponse). If this field may contain data URLs with sensitive information, consider adding validation to prevent them from being stored, consistent with patterns used elsewhere in the codebase.relay/channel/task/suno/adaptor.go (1)
107-110: LGTM!The early return pattern for official URL handling is clean and properly delegates to
DoResponseOfficial.relay/channel/task/suno/suno_official.go (1)
21-32: LGTM!The struct definitions for
OfficialSunoData,OfficialSunoResponseData, andOfficialSunoResponseappropriately model the official Suno API response structure with proper JSON tags and pointer types for optional fields.Also applies to: 35-48, 51-55
controller/task.go (1)
167-169: Time unit conversion assumes milliseconds from official API.The code divides timestamps by 1000 to convert from milliseconds to seconds. Ensure this is consistent with both official and non-official API responses. If non-official APIs already return seconds, this division would corrupt the data. Verify that both API paths return timestamps in milliseconds before deployment.
| baseUrl := channel.GetBaseURL() | ||
| resp, err := adaptor.FetchTask(baseUrl, channel.Key, map[string]any{ |
There was a problem hiding this comment.
Inconsistent BaseURL access: potential nil pointer dereference.
Line 120 correctly uses channel.GetBaseURL() (likely a safe accessor), but line 142 directly dereferences *channel.BaseURL. If BaseURL is nil, this will cause a panic.
Use consistent access pattern:
// 判断是否是官方 Suno API
- if constant.IsOfficialUrl(*channel.BaseURL) {
+ if constant.IsOfficialUrl(channel.GetBaseURL()) {Also applies to: 142-142
🤖 Prompt for AI Agents
In controller/task.go around lines 120-121 and 142, the code uses
channel.GetBaseURL() at line 120 but directly dereferences *channel.BaseURL at
line 142 which can panic if BaseURL is nil; replace the direct dereference with
the safe accessor (channel.GetBaseURL()) or first nil-check channel.BaseURL
before dereferencing, and ensure both places consistently use the getter so no
nil pointer dereference can occur.
| if sunoRequest.CallBackUrl == "" { | ||
| // 设置默认回调地址, api不准为空, 先预留 | ||
| sunoRequest.CallBackUrl = fmt.Sprintf("%s/suno/callback", system_setting.ServerAddress) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for suno/callback endpoint implementation
echo "=== Searching for /suno/callback handler ==="
rg -n "suno/callback" --type go -A 3 -B 3
echo ""
echo "=== Searching for callback route registration ==="
rg -n "callback" --type go | grep -i "suno\|route\|handler" | head -20Repository: QuantumNous/new-api
Length of output: 182
Default callback URL implementation is incomplete and may expose internal server address.
The code sets a default callback URL using system_setting.ServerAddress to send to the external Suno API, but the associated comment "api不准为空, 先预留" (API doesn't allow empty, reserved for now) indicates the /suno/callback endpoint is not yet implemented. Before merging:
- Implement the
/suno/callbackhandler to receive and process callbacks - Evaluate whether exposing
ServerAddressto external services aligns with security requirements, or use a public/proxy URL instead
🤖 Prompt for AI Agents
In relay/channel/task/suno/adaptor.go around lines 46-49, the default callback
URL is set to an internal ServerAddress and the comment indicates the
/suno/callback endpoint is not implemented; implement a HTTP handler for
/suno/callback that validates and processes incoming Suno callbacks (parse and
verify payload, handle success/error events, respond 200/4xx appropriately, and
log securely), and replace or gate use of system_setting.ServerAddress by
requiring a configured public callback URL (from env/config) or a proxied public
endpoint; ensure the selected URL is validated (must be https and in allowlist)
before sending it to the external API and update configuration docs accordingly.
| func (a *TaskAdaptor) FetchTaskOfficial(baseUrl, key string, body map[string]any) (*http.Response, error) { | ||
| ids, ok := body["ids"].([]string) | ||
| if !ok || len(ids) == 0 { | ||
| return nil, fmt.Errorf("ids array is required in body") | ||
| } | ||
|
|
||
| taskId := ids[0] | ||
| if taskId == "" { | ||
| return nil, fmt.Errorf("taskId cannot be empty") | ||
| } | ||
|
|
||
| requestUrl := fmt.Sprintf("%s/api/v1/generate/record-info?taskId=%s", baseUrl, taskId) | ||
|
|
||
| req, err := http.NewRequest("GET", requestUrl, nil) | ||
| if err != nil { | ||
| common.SysLog(fmt.Sprintf("Get Task error: %v", err)) | ||
| return nil, err | ||
| } | ||
|
|
||
| // 使用带有超时的 context 创建新的请求 | ||
| req = req.WithContext(context.TODO()) | ||
| req.Header.Set("Authorization", "Bearer "+key) | ||
| resp, err := service.GetHttpClient().Do(req) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return resp, nil | ||
| } |
There was a problem hiding this comment.
FetchTaskOfficial only processes the first task ID, ignoring the rest.
The non-official FetchTask in adaptor.go sends all task IDs in the body for batch processing. However, FetchTaskOfficial extracts only ids[0] and ignores the remaining IDs. This breaks batch task updates for official API users.
The official API may not support batch fetching in the same way. If so, you need to either:
- Loop over all IDs and make multiple requests, or
- Document this limitation and handle it in the calling code (
updateSunoTaskAll).
func (a *TaskAdaptor) FetchTaskOfficial(baseUrl, key string, body map[string]any) (*http.Response, error) {
ids, ok := body["ids"].([]string)
if !ok || len(ids) == 0 {
return nil, fmt.Errorf("ids array is required in body")
}
- taskId := ids[0]
- if taskId == "" {
- return nil, fmt.Errorf("taskId cannot be empty")
+ // TODO: Official API only supports single task query per request.
+ // The caller should handle batching if needed, or loop here.
+ if len(ids) > 1 {
+ common.SysLog("Warning: FetchTaskOfficial only fetches the first task, ignoring remaining IDs")
}
+ taskId := ids[0]Committable suggestion skipped: line range outside the PR's diff.
| // 使用带有超时的 context 创建新的请求 | ||
| req = req.WithContext(context.TODO()) | ||
| req.Header.Set("Authorization", "Bearer "+key) |
There was a problem hiding this comment.
Missing timeout for HTTP request.
The non-official FetchTask in adaptor.go (lines 169-174) sets a 15-second timeout. However, FetchTaskOfficial uses context.TODO() without any timeout, which could cause the request to hang indefinitely.
Add a timeout consistent with the non-official implementation:
+ timeout := time.Second * 15
+ ctx, cancel := context.WithTimeout(context.Background(), timeout)
+ defer cancel()
+
req, err := http.NewRequest("GET", requestUrl, nil)
if err != nil {
common.SysLog(fmt.Sprintf("Get Task error: %v", err))
return nil, err
}
// 使用带有超时的 context 创建新的请求
- req = req.WithContext(context.TODO())
+ req = req.WithContext(ctx)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In relay/channel/task/suno/suno_official.go around lines 156 to 158, the request
uses context.TODO() so the HTTP call can hang indefinitely; replace
context.TODO() with a context that has a 15-second timeout (matching the
non-official FetchTask), e.g., create ctx, cancel :=
context.WithTimeout(context.Background(), 15*time.Second), call req =
req.WithContext(ctx), and ensure you call cancel() (defer cancel()) to avoid a
context leak before sending the request; keep the existing Authorization header
logic unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
controller/task.go (2)
20-20: 官方/非官方 Suno 分支解析整体合理，但建议确认错误处理和日志一致性
- 使用
suno.ParseResponseItems处理官方 Suno 响应，并保持旧的json.Unmarshal路径作为非官方分支，这个分层设计清晰、可维护。- 非官方路径在
!responseItems.IsSuccess()时会记录完整 body 并返回业务错误；官方路径目前只在ParseResponseItems返回 error 时中止。建议：
- 确认
suno.ParseResponseItems内部已经对code/msg做了和旧逻辑等价的校验与日志（包括记录原始responseBody）。- 如果没有，建议在官方分支也补充一次
IsSuccess判定及包含 body 的错误日志，以便排查线上问题时两条路径的 observability 一致。如果
ParseResponseItems已经封装了这些逻辑，可以忽略上述建议。Also applies to: 139-157
166-169: 为FailReason增加 data: URL 过滤以避免写入超长 base64 数据这里直接把
responseItem.FailReason写入task.FailReason：task.FailReason = lo.If(responseItem.FailReason != "", responseItem.FailReason).Else(task.FailReason)根据之前在视频任务里约定的模式，
FailReason里如果出现以data:开头的 base64 URL，会导致异常大的文本落库（既浪费存储，也影响查询/展示）。建议在赋值前做一次前缀过滤：-import ( +import ( "context" "encoding/json" "errors" "fmt" "io" "net/http" "sort" "strconv" "time" + "strings"- task.FailReason = lo.If(responseItem.FailReason != "", responseItem.FailReason).Else(task.FailReason) + failReason := responseItem.FailReason + // 避免把 data: URL 形式的超长 base64 错误信息写入数据库 + if strings.HasPrefix(failReason, "data:") { + failReason = "" + } + task.FailReason = lo.If(failReason != "", failReason).Else(task.FailReason)这样可以在 Suno 场景下也复用统一的 “不落 data: URL 到 FailReason” 规范。
Based on learnings, this pattern is expected to be applied consistently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
constant/channel.go(2 hunks)controller/task.go(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- constant/channel.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.
Applied to files:
controller/task.go
🔇 Additional comments (1)
controller/task.go (1)
120-123: 统一使用GetBaseURL避免 BaseURL 为空导致 panic这里通过
baseUrl := channel.GetBaseURL()，在FetchTask和IsOfficialTypeChannel中复用同一个值，替代之前直接解引用*channel.BaseURL的方式，消除了潜在的空指针风险，这个修正是合理的。Also applies to: 142-143
| task.SubmitTime = lo.If(responseItem.SubmitTime != 0, responseItem.SubmitTime/1000).Else(task.SubmitTime) | ||
| task.StartTime = lo.If(responseItem.StartTime != 0, responseItem.StartTime/1000).Else(task.StartTime) | ||
| task.FinishTime = lo.If(responseItem.FinishTime != 0, responseItem.FinishTime/1000).Else(task.FinishTime) |
There was a problem hiding this comment.
时间戳单位不一致导致每次轮询都会判定“需要更新”
现在：
- 持久化时：
task.SubmitTime/StartTime/FinishTime被设为responseItem.*Time / 1000（秒）。 - 但在
checkTaskNeedUpdate中，仍然直接用oldTask.*Time（秒）与newTask.*Time（毫秒）比较：
if oldTask.SubmitTime != newTask.SubmitTime { ... }
if oldTask.StartTime != newTask.StartTime { ... }
if oldTask.FinishTime != newTask.FinishTime { ... }这会导致：
- 首次轮询会更新（预期行为）。
- 之后每一轮，因为秒与毫秒始终不相等，
checkTaskNeedUpdate永远返回true，造成对同一任务的重复UPDATE，增加无谓的 DB 压力，也让“是否真的有变化”这一判断失效。
建议在比较前统一时间单位，例如在 checkTaskNeedUpdate 中将 newTask 的时间戳也转换为秒再比较：
func checkTaskNeedUpdate(oldTask *model.Task, newTask dto.SunoDataResponse) bool {
-
- if oldTask.SubmitTime != newTask.SubmitTime {
+ // Suno 返回毫秒时间戳，oldTask 持久化为秒，这里统一到秒再比较
+ newSubmitTime := newTask.SubmitTime
+ if newSubmitTime != 0 {
+ newSubmitTime /= 1000
+ }
+ newStartTime := newTask.StartTime
+ if newStartTime != 0 {
+ newStartTime /= 1000
+ }
+ newFinishTime := newTask.FinishTime
+ if newFinishTime != 0 {
+ newFinishTime /= 1000
+ }
+
+ if oldTask.SubmitTime != newSubmitTime {
return true
}
- if oldTask.StartTime != newTask.StartTime {
+ if oldTask.StartTime != newStartTime {
return true
}
- if oldTask.FinishTime != newTask.FinishTime {
+ if oldTask.FinishTime != newFinishTime {
return true
}这样既保持 DB 中存秒的设计，又能避免无意义的重复更新。顺带一提，成功分支里新增的 task.Url = responseItem.Url 也能在第一次状态成功时被正确持久化。
Also applies to: 202-212
There was a problem hiding this comment.
好的, 已经将suno返回的毫秒转换成秒, 统一以秒为单位
There was a problem hiding this comment.
@feitianbubu 感谢你的更新！我看到你已经在存储时将时间戳从毫秒转换为秒（Lines 165-167），这部分很好。👍
不过，还需要同步更新 checkTaskNeedUpdate 函数（Lines 196-230），因为它目前仍然在直接比较：
oldTask.*Time（数据库中的秒）newTask.*Time（来自dto.SunoDataResponse，仍然是毫秒）
这会导致即使任务没有实际变化，每次轮询也会因为单位不同而判定"需要更新"。
建议在 checkTaskNeedUpdate 开头添加转换逻辑：
func checkTaskNeedUpdate(oldTask *model.Task, newTask dto.SunoDataResponse) bool {
// 将 newTask 的毫秒时间戳转换为秒，与 oldTask 保持一致
newSubmitTime := newTask.SubmitTime
if newSubmitTime != 0 {
newSubmitTime /= 1000
}
newStartTime := newTask.StartTime
if newStartTime != 0 {
newStartTime /= 1000
}
newFinishTime := newTask.FinishTime
if newFinishTime != 0 {
newFinishTime /= 1000
}
if oldTask.SubmitTime != newSubmitTime {
return true
}
if oldTask.StartTime != newStartTime {
return true
}
if oldTask.FinishTime != newFinishTime {
return true
}
// ... 其余比较逻辑保持不变
}这样就能确保比较时单位一致，避免无意义的数据库更新。
| } | ||
|
|
||
| // 使用带有超时的 context 创建新的请求 | ||
| req = req.WithContext(context.TODO()) |
There was a problem hiding this comment.
已提交新commit, 去除了context.TOTO()超时定义代码, 默认继承自service client的超时逻辑更简洁
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
relay/channel/task/suno/suno_official.go (2)
142-145: Batch task fetching not supported - only first ID processed.This issue was previously flagged but remains unresolved. The function extracts only
ids[0]and ignores remaining IDs, which breaks batch task updates for official API users.If the official API doesn't support batch queries, you must either:
- Loop over all IDs and make separate requests, or
- Document this limitation and handle it in the caller
Add a warning when multiple IDs are provided:
+ // Official API only supports single task query per request + if len(ids) > 1 { + common.SysLog("Warning: FetchTaskOfficial only fetches the first task, ignoring remaining IDs") + } + taskId := ids[0] if taskId == "" { return nil, fmt.Errorf("taskId cannot be empty") }
149-160: Missing timeout for HTTP request - can hang indefinitely.This issue was previously flagged but remains unresolved. The request is created without any timeout context, which could cause it to hang indefinitely. The non-official
FetchTaskimplementation uses a 15-second timeout.Apply this diff to add a timeout:
+ timeout := time.Second * 15 + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + req, err := http.NewRequest("GET", requestUrl, nil) if err != nil { common.SysLog(fmt.Sprintf("Get Task error: %v", err)) return nil, err } + req = req.WithContext(ctx) req.Header.Set("Authorization", "Bearer "+key) resp, err := service.GetHttpClient().Do(req)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relay/channel/task/suno/suno_official.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.
Applied to files:
relay/channel/task/suno/suno_official.go
📚 Learning: 2025-06-15T12:38:11.806Z
Learnt from: feitianbubu
Repo: QuantumNous/new-api PR: 1228
File: router/main.go:28-36
Timestamp: 2025-06-15T12:38:11.806Z
Learning: gin.Context implements context.Context interface since Gin v1.8.0, providing the methods Deadline(), Done(), Err(), and Value(). When using Gin v1.8.0 or later, gin.Context can be passed directly to functions expecting context.Context without needing to extract c.Request.Context().
Applied to files:
relay/channel/task/suno/suno_official.go
🧬 Code graph analysis (1)
relay/channel/task/suno/suno_official.go (5)
dto/suno.go (2)
TaskResponse(76-80)SunoDataResponse(31-41)common/json.go (2)
Marshal(21-23)Unmarshal(9-11)service/error.go (1)
TaskErrorWrapper(140-157)common/sys_log.go (1)
SysLog(11-14)service/http_client.go (1)
GetHttpClient(49-51)
🔇 Additional comments (6)
relay/channel/task/suno/suno_official.go (6)
1-17: LGTM!The imports are appropriate for the official Suno API integration functionality.
19-31: LGTM!The struct properly models the official Suno API data with appropriate JSON tags and field types.
33-47: LGTM!The struct correctly uses pointers for optional error fields and properly models the nested response structure.
49-58: LGTM!The response structure and success check are correctly implemented.
100-133: LGTM!The method properly handles the official API response: reads and unmarshals the body, validates success, forwards headers, and streams the response to the client.
163-175: LGTM!The function properly parses the official API response, validates success, and converts it to the standard format with appropriate error wrapping.
| // 将官方数据转换为 JSON 存储 | ||
| var dataBytes []byte | ||
| if len(r.Data.Response.SunoData) > 0 { | ||
| dataBytes, _ = json.Marshal(r.Data.Response.SunoData) | ||
| } |
There was a problem hiding this comment.
Handle the json.Marshal error.
The error from json.Marshal is silently ignored, which could hide serialization failures. While unlikely, if the data contains non-serializable values, this will silently produce empty data.
Apply this diff:
// 将官方数据转换为 JSON 存储
var dataBytes []byte
if len(r.Data.Response.SunoData) > 0 {
- dataBytes, _ = json.Marshal(r.Data.Response.SunoData)
+ dataBytes, err := json.Marshal(r.Data.Response.SunoData)
+ if err != nil {
+ common.SysLog(fmt.Sprintf("Failed to marshal SunoData: %v", 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 将官方数据转换为 JSON 存储 | |
| var dataBytes []byte | |
| if len(r.Data.Response.SunoData) > 0 { | |
| dataBytes, _ = json.Marshal(r.Data.Response.SunoData) | |
| } | |
| // 将官方数据转换为 JSON 存储 | |
| var dataBytes []byte | |
| if len(r.Data.Response.SunoData) > 0 { | |
| dataBytes, err := json.Marshal(r.Data.Response.SunoData) | |
| if err != nil { | |
| common.SysLog(fmt.Sprintf("Failed to marshal SunoData: %v", err)) | |
| } | |
| } |
🤖 Prompt for AI Agents
In relay/channel/task/suno/suno_official.go around lines 76 to 80, json.Marshal
is called and its error ignored; change it to capture the error (e.g. dataBytes,
err := json.Marshal(...)) and handle failures instead of discarding them — if
Marshal returns an error, either return it up the call chain or log the error
and skip/abort storing the data; only assign/use dataBytes when err == nil so
serialization failures are not silently ignored.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
controller/task.go (1)
202-221: Time unit mismatch causes unnecessary database updates on every poll.Per the past review comment (still applicable): the official Suno API returns timestamps in milliseconds, but they are stored in seconds after division. However,
checkTaskNeedUpdatecomparesoldTask.*Time(seconds) directly withnewTask.*Time(milliseconds from the official API), causing the function to always returntrue.Additionally, lines 219-221 duplicate the check already performed at lines 210-212.
Apply this diff to normalize time units and remove the duplicate check:
func checkTaskNeedUpdate(oldTask *model.Task, newTask dto.SunoDataResponse) bool { + // Normalize timestamps to seconds for comparison (official API returns ms) + newSubmitTime := newTask.SubmitTime + newStartTime := newTask.StartTime + newFinishTime := newTask.FinishTime - if oldTask.SubmitTime != newTask.SubmitTime { + if oldTask.SubmitTime != newSubmitTime { return true } - if oldTask.StartTime != newTask.StartTime { + if oldTask.StartTime != newStartTime { return true } - if oldTask.FinishTime != newTask.FinishTime { + if oldTask.FinishTime != newFinishTime { return true } if string(oldTask.Status) != newTask.Status { return true } if oldTask.FailReason != newTask.FailReason { return true } - if oldTask.FinishTime != newTask.FinishTime { - return true - }Note: The
ToStandardResponse()insuno_official.goalready divides timestamps by 1000 before returning, so normalization may already be handled there. Verify whetherParseResponseItemsreturns seconds or milliseconds to confirm if this comparison is correct.relay/channel/task/suno/suno_official.go (2)
76-80: Handle thejson.Marshalerror.The error from
json.Marshalis silently ignored. While unlikely to fail for this data, it's better to log failures for debugging.// 将官方数据转换为 JSON 存储 var dataBytes []byte if len(r.Data.Response.SunoData) > 0 { - dataBytes, _ = json.Marshal(r.Data.Response.SunoData) + var err error + dataBytes, err = json.Marshal(r.Data.Response.SunoData) + if err != nil { + common.SysLog(fmt.Sprintf("Failed to marshal SunoData: %v", err)) + } }
136-161:FetchTaskOfficialonly fetches the first task ID, ignoring the rest.The non-official
FetchTasksends all task IDs for batch processing, but this method only usesids[0]. This breaks batch task updates for official API users, causing incomplete polling.If the official API doesn't support batch queries, consider:
- Having the caller loop over IDs individually, or
- Adding a warning log to make this limitation visible:
func (a *TaskAdaptor) FetchTaskOfficial(baseUrl, key string, body map[string]any) (*http.Response, error) { ids, ok := body["ids"].([]string) if !ok || len(ids) == 0 { return nil, fmt.Errorf("ids array is required in body") } + if len(ids) > 1 { + common.SysLog(fmt.Sprintf("Warning: FetchTaskOfficial only fetches first task, ignoring %d remaining IDs", len(ids)-1)) + } + taskId := ids[0]
🧹 Nitpick comments (1)
relay/channel/task/suno/suno_official.go (1)
67-74:finishTimeuses current server time instead of API response time.When status is
SUCCESS,finishTimeis set totime.Now().Unix(). This may not accurately reflect when the task actually finished on Suno's servers, especially if there's polling delay.Consider using a timestamp from the response if available, or document this as an approximation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
controller/task.go(4 hunks)relay/channel/task/suno/suno_official.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.
Applied to files:
controller/task.gorelay/channel/task/suno/suno_official.go
📚 Learning: 2025-06-15T12:38:11.806Z
Learnt from: feitianbubu
Repo: QuantumNous/new-api PR: 1228
File: router/main.go:28-36
Timestamp: 2025-06-15T12:38:11.806Z
Learning: gin.Context implements context.Context interface since Gin v1.8.0, providing the methods Deadline(), Done(), Err(), and Value(). When using Gin v1.8.0 or later, gin.Context can be passed directly to functions expecting context.Context without needing to extract c.Request.Context().
Applied to files:
relay/channel/task/suno/suno_official.go
🧬 Code graph analysis (1)
relay/channel/task/suno/suno_official.go (6)
dto/suno.go (2)
TaskResponse(76-80)SunoDataResponse(31-41)common/json.go (2)
Marshal(21-23)Unmarshal(9-11)relay/channel/task/suno/adaptor.go (1)
TaskAdaptor(25-27)service/error.go (1)
TaskErrorWrapper(140-157)common/sys_log.go (1)
SysLog(11-14)service/http_client.go (1)
GetHttpClient(49-51)
🔇 Additional comments (7)
controller/task.go (3)
120-121: LGTM!The previous nil pointer dereference issue has been addressed. Using
channel.GetBaseURL()and storing it in a local variable ensures consistent and safe access throughout the function.
141-157: Branching logic for official vs non-official API looks correct.The official path uses
suno.ParseResponseItemswhile non-official uses standard JSON unmarshaling. Error handling is appropriate for each branch.
188-191: LGTM!Setting
task.Urlfrom the response on success enables URL storage for the official API integration.relay/channel/task/suno/suno_official.go (4)
19-31: LGTM!The
OfficialSunoDatastruct correctly maps the official Suno API response fields with appropriate JSON tags.
33-58: LGTM!The response data structures and
IsSuccess()method are well-defined for the official API integration.
100-133: LGTM!
DoResponseOfficialproperly handles response parsing, error wrapping, and header forwarding. The implementation correctly returns early on failures and forwards the response to the client.
163-175: LGTM!
ParseResponseItemscorrectly parses the official response and converts it to the standard format viaToStandardResponse(). Error handling includes helpful context in the wrapped error message.
The merge-base changed after approval.
19 participants
官方文档: https://docs.sunoapi.org/cn/suno-api/generate-music
支持歌曲生成, 任务查询
一. 歌曲生成
请求示例:
返回示例:
查询示例: 和原来一致
Summary by CodeRabbit
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.