From ed10064748101088290170faa3bf8115e4c38c84 Mon Sep 17 00:00:00 2001 From: dylanhuff-at-coder Date: Wed, 5 Aug 2026 17:41:18 -0400 Subject: [PATCH] refactor(codersdk): use shared error helpers in chat endpoints (#27858) Migrate chat endpoint response decoding to `ReadBodyAsJSON` and consolidate `ReadBodyAsError` construction through `newResponseError`, so empty-body and non-JSON errors consistently include the request method and URL. Stacked on #27857, with the lint rule following in #27859. Refs #27044. Reviewed and updated by Coder Agents on behalf of @dylanhuff-at-coder. --- codersdk/chats.go | 98 ++++++++++++++++---------------- codersdk/client.go | 91 ++++++++++++++--------------- codersdk/client_internal_test.go | 8 ++- 3 files changed, 100 insertions(+), 97 deletions(-) diff --git a/codersdk/chats.go b/codersdk/chats.go index f6f2e1540c..0eb2e88499 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -2008,7 +2008,7 @@ func (c *ExperimentalClient) ListChats(ctx context.Context, opts *ListChatsOptio return nil, ReadBodyAsError(res) } var chats []Chat - return chats, json.NewDecoder(res.Body).Decode(&chats) + return chats, ReadBodyAsJSON(res, &chats) } // ListChatModels returns the available chat model catalog. @@ -2023,7 +2023,7 @@ func (c *ExperimentalClient) ListChatModels(ctx context.Context) (ChatModelsResp } var catalog ChatModelsResponse - return catalog, json.NewDecoder(res.Body).Decode(&catalog) + return catalog, ReadBodyAsJSON(res, &catalog) } // ListChatProviders returns admin-managed chat provider configs. @@ -2038,7 +2038,7 @@ func (c *ExperimentalClient) ListChatProviders(ctx context.Context) ([]ChatProvi } var providers []ChatProviderConfig - return providers, json.NewDecoder(res.Body).Decode(&providers) + return providers, ReadBodyAsJSON(res, &providers) } // CreateChatProvider creates an admin-managed chat provider config. @@ -2053,7 +2053,7 @@ func (c *ExperimentalClient) CreateChatProvider(ctx context.Context, req CreateC } var provider ChatProviderConfig - return provider, json.NewDecoder(res.Body).Decode(&provider) + return provider, ReadBodyAsJSON(res, &provider) } // UpdateChatProvider updates an admin-managed chat provider config. @@ -2068,7 +2068,7 @@ func (c *ExperimentalClient) UpdateChatProvider(ctx context.Context, providerID } var provider ChatProviderConfig - return provider, json.NewDecoder(res.Body).Decode(&provider) + return provider, ReadBodyAsJSON(res, &provider) } // DeleteChatProvider deletes an admin-managed chat provider config. @@ -2095,7 +2095,7 @@ func (c *ExperimentalClient) ListUserAIProviderKeyConfigs(ctx context.Context, u return nil, ReadBodyAsError(res) } var configs []UserAIProviderKeyConfig - return configs, json.NewDecoder(res.Body).Decode(&configs) + return configs, ReadBodyAsJSON(res, &configs) } // UpsertUserAIProviderKey creates or replaces a user API key for an AI provider. @@ -2109,7 +2109,7 @@ func (c *ExperimentalClient) UpsertUserAIProviderKey(ctx context.Context, user s return UserAIProviderKeyConfig{}, ReadBodyAsError(res) } var config UserAIProviderKeyConfig - return config, json.NewDecoder(res.Body).Decode(&config) + return config, ReadBodyAsJSON(res, &config) } // DeleteUserAIProviderKey deletes a user API key for an AI provider. @@ -2140,7 +2140,7 @@ func (c *ExperimentalClient) ListUserChatProviderConfigs(ctx context.Context) ([ return nil, ReadBodyAsError(res) } var configs []UserChatProviderConfig - return configs, json.NewDecoder(res.Body).Decode(&configs) + return configs, ReadBodyAsJSON(res, &configs) } // UpsertUserChatProviderKey creates or replaces a user API key for a provider. @@ -2154,7 +2154,7 @@ func (c *ExperimentalClient) UpsertUserChatProviderKey(ctx context.Context, prov return UserChatProviderConfig{}, ReadBodyAsError(res) } var config UserChatProviderConfig - return config, json.NewDecoder(res.Body).Decode(&config) + return config, ReadBodyAsJSON(res, &config) } // DeleteUserChatProviderKey deletes a user API key for a provider. @@ -2182,7 +2182,7 @@ func (c *ExperimentalClient) ListChatModelConfigs(ctx context.Context) ([]ChatMo } var configs []ChatModelConfig - return configs, json.NewDecoder(res.Body).Decode(&configs) + return configs, ReadBodyAsJSON(res, &configs) } // CreateChatModelConfig creates an admin-managed chat model config. @@ -2197,7 +2197,7 @@ func (c *ExperimentalClient) CreateChatModelConfig(ctx context.Context, req Crea } var config ChatModelConfig - return config, json.NewDecoder(res.Body).Decode(&config) + return config, ReadBodyAsJSON(res, &config) } // UpdateChatModelConfig updates an admin-managed chat model config. @@ -2212,7 +2212,7 @@ func (c *ExperimentalClient) UpdateChatModelConfig(ctx context.Context, modelCon } var config ChatModelConfig - return config, json.NewDecoder(res.Body).Decode(&config) + return config, ReadBodyAsJSON(res, &config) } // DeleteChatModelConfig deletes an admin-managed chat model config. @@ -2240,7 +2240,7 @@ func (c *ExperimentalClient) GetChatCost(ctx context.Context, chatID uuid.UUID) return ChatCost{}, ReadBodyAsError(res) } var cost ChatCost - return cost, json.NewDecoder(res.Body).Decode(&cost) + return cost, ReadBodyAsJSON(res, &cost) } // GetChatSystemPrompt returns the deployment-wide chat system prompt. @@ -2254,7 +2254,7 @@ func (c *ExperimentalClient) GetChatSystemPrompt(ctx context.Context) (ChatSyste return ChatSystemPromptResponse{}, ReadBodyAsError(res) } var resp ChatSystemPromptResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatSystemPrompt updates the deployment-wide chat system prompt. @@ -2281,7 +2281,7 @@ func (c *ExperimentalClient) GetChatPlanModeInstructions(ctx context.Context) (C return ChatPlanModeInstructionsResponse{}, ReadBodyAsError(res) } var resp ChatPlanModeInstructionsResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatPlanModeInstructions updates the deployment-wide plan mode instructions. @@ -2313,7 +2313,7 @@ func (c *ExperimentalClient) GetChatModelOverride(ctx context.Context, override return ChatModelOverrideResponse{}, ReadBodyAsError(res) } var resp ChatModelOverrideResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatModelOverride updates the deployment-wide chat model override for @@ -2346,7 +2346,7 @@ func (c *ExperimentalClient) GetChatPersonalModelOverridesAdminSettings(ctx cont return ChatPersonalModelOverridesAdminSettings{}, ReadBodyAsError(res) } var resp ChatPersonalModelOverridesAdminSettings - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatPersonalModelOverridesAdminSettings updates the deployment-wide @@ -2375,7 +2375,7 @@ func (c *ExperimentalClient) GetUserChatPersonalModelOverrides(ctx context.Conte return UserChatPersonalModelOverridesResponse{}, ReadBodyAsError(res) } var resp UserChatPersonalModelOverridesResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateUserChatPersonalModelOverride updates the user's personal model @@ -2407,7 +2407,7 @@ func (c *ExperimentalClient) GetUserChatCustomPrompt(ctx context.Context) (UserC return UserChatCustomPrompt{}, ReadBodyAsError(res) } var resp UserChatCustomPrompt - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // GetChatAdvisorConfig returns the deployment-wide advisor configuration. @@ -2421,7 +2421,7 @@ func (c *ExperimentalClient) GetChatAdvisorConfig(ctx context.Context) (AdvisorC return AdvisorConfig{}, ReadBodyAsError(res) } var resp AdvisorConfig - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatAdvisorConfig updates the deployment-wide advisor configuration. @@ -2448,7 +2448,7 @@ func (c *ExperimentalClient) GetChatComputerUseProvider(ctx context.Context) (Ch return ChatComputerUseProviderResponse{}, ReadBodyAsError(res) } var resp ChatComputerUseProviderResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatComputerUseProvider updates the deployment-wide computer use @@ -2476,7 +2476,7 @@ func (c *ExperimentalClient) GetChatWorkspaceTTL(ctx context.Context) (ChatWorks return ChatWorkspaceTTLResponse{}, ReadBodyAsError(res) } var resp ChatWorkspaceTTLResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatWorkspaceTTL updates the chat workspace TTL setting. @@ -2503,7 +2503,7 @@ func (c *ExperimentalClient) GetChatRetentionDays(ctx context.Context) (ChatRete return ChatRetentionDaysResponse{}, ReadBodyAsError(res) } var resp ChatRetentionDaysResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatRetentionDays updates the chat retention period. @@ -2531,7 +2531,7 @@ func (c *ExperimentalClient) GetChatDebugRetentionDays(ctx context.Context) (Cha return ChatDebugRetentionDaysResponse{}, ReadBodyAsError(res) } var resp ChatDebugRetentionDaysResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatDebugRetentionDays updates the chat debug run retention period. @@ -2558,7 +2558,7 @@ func (c *ExperimentalClient) GetChatAutoArchiveDays(ctx context.Context) (ChatAu return ChatAutoArchiveDaysResponse{}, ReadBodyAsError(res) } var resp ChatAutoArchiveDaysResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatAutoArchiveDays updates the chat auto-archive period. @@ -2585,7 +2585,7 @@ func (c *ExperimentalClient) GetChatTemplateAllowlist(ctx context.Context) (Chat return ChatTemplateAllowlist{}, ReadBodyAsError(res) } var resp ChatTemplateAllowlist - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatTemplateAllowlist updates the deployment-wide chat template allowlist. @@ -2612,7 +2612,7 @@ func (c *ExperimentalClient) UpdateUserChatCustomPrompt(ctx context.Context, req return UserChatCustomPrompt{}, ReadBodyAsError(res) } var resp UserChatCustomPrompt - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // GetUserChatCompactionThresholds fetches the user's per-model chat @@ -2627,7 +2627,7 @@ func (c *ExperimentalClient) GetUserChatCompactionThresholds(ctx context.Context return UserChatCompactionThresholds{}, ReadBodyAsError(res) } var thresholds UserChatCompactionThresholds - return thresholds, json.NewDecoder(res.Body).Decode(&thresholds) + return thresholds, ReadBodyAsJSON(res, &thresholds) } // UpdateUserChatCompactionThreshold updates the user's per-model chat @@ -2642,7 +2642,7 @@ func (c *ExperimentalClient) UpdateUserChatCompactionThreshold(ctx context.Conte return UserChatCompactionThreshold{}, ReadBodyAsError(res) } var threshold UserChatCompactionThreshold - return threshold, json.NewDecoder(res.Body).Decode(&threshold) + return threshold, ReadBodyAsJSON(res, &threshold) } // DeleteUserChatCompactionThreshold deletes the user's per-model chat @@ -2670,7 +2670,7 @@ func (c *ExperimentalClient) CreateChat(ctx context.Context, req CreateChatReque } defer res.Body.Close() var chat Chat - return chat, json.NewDecoder(res.Body).Decode(&chat) + return chat, ReadBodyAsJSON(res, &chat) } // StreamChatOptions are optional parameters for StreamChat. @@ -2824,7 +2824,7 @@ func (c *ExperimentalClient) GetChatDebugLogging(ctx context.Context) (ChatDebug return ChatDebugLoggingAdminSettings{}, ReadBodyAsError(res) } var resp ChatDebugLoggingAdminSettings - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChatDebugLogging updates the runtime admin setting that allows @@ -2853,7 +2853,7 @@ func (c *ExperimentalClient) GetUserChatDebugLogging(ctx context.Context) (UserC return UserChatDebugLoggingSettings{}, ReadBodyAsError(res) } var resp UserChatDebugLoggingSettings - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateUserChatDebugLogging updates the current user's chat debug @@ -2881,7 +2881,7 @@ func (c *ExperimentalClient) GetChatDebugRuns(ctx context.Context, chatID uuid.U return nil, ReadBodyAsError(res) } var resp []ChatDebugRunSummary - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // GetChatDebugRun returns a single debug run along with its full step @@ -2896,7 +2896,7 @@ func (c *ExperimentalClient) GetChatDebugRun(ctx context.Context, chatID uuid.UU return ChatDebugRun{}, ReadBodyAsError(res) } var resp ChatDebugRun - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // GetChat returns a chat by ID. @@ -2910,7 +2910,7 @@ func (c *ExperimentalClient) GetChat(ctx context.Context, chatID uuid.UUID) (Cha return Chat{}, ReadBodyAsError(res) } var chat Chat - return chat, json.NewDecoder(res.Body).Decode(&chat) + return chat, ReadBodyAsJSON(res, &chat) } // RefreshChatContext re-pins the chat to its agent's latest context snapshot @@ -2925,7 +2925,7 @@ func (c *ExperimentalClient) RefreshChatContext(ctx context.Context, chatID uuid return Chat{}, ReadBodyAsError(res) } var chat Chat - return chat, json.NewDecoder(res.Body).Decode(&chat) + return chat, ReadBodyAsJSON(res, &chat) } func (c *ExperimentalClient) GetChatACL(ctx context.Context, chatID uuid.UUID) (ChatACL, error) { @@ -2938,7 +2938,7 @@ func (c *ExperimentalClient) GetChatACL(ctx context.Context, chatID uuid.UUID) ( return ChatACL{}, ReadBodyAsError(res) } var acl ChatACL - return acl, json.NewDecoder(res.Body).Decode(&acl) + return acl, ReadBodyAsJSON(res, &acl) } func (c *ExperimentalClient) UpdateChatACL(ctx context.Context, chatID uuid.UUID, req UpdateChatACL) error { @@ -2995,7 +2995,7 @@ func (c *ExperimentalClient) GetChatMessages(ctx context.Context, chatID uuid.UU return ChatMessagesResponse{}, ReadBodyAsError(res) } var resp ChatMessagesResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // ChatPromptsOptions are optional query parameters for GetChatPrompts. @@ -3030,7 +3030,7 @@ func (c *ExperimentalClient) GetChatPrompts(ctx context.Context, chatID uuid.UUI return ChatPromptsResponse{}, ReadBodyAsError(res) } var resp ChatPromptsResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // UpdateChat patches a chat resource. @@ -3057,7 +3057,7 @@ func (c *ExperimentalClient) CreateChatMessage(ctx context.Context, chatID uuid. } defer res.Body.Close() var resp CreateChatMessageResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // EditChatMessage edits an existing user message in a chat and re-runs from there. @@ -3081,7 +3081,7 @@ func (c *ExperimentalClient) EditChatMessage( } defer res.Body.Close() var resp EditChatMessageResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // InterruptChat cancels an in-flight chat run and leaves it waiting. @@ -3095,7 +3095,7 @@ func (c *ExperimentalClient) InterruptChat(ctx context.Context, chatID uuid.UUID return Chat{}, ReadBodyAsError(res) } var chat Chat - return chat, json.NewDecoder(res.Body).Decode(&chat) + return chat, ReadBodyAsJSON(res, &chat) } // CompactChat requests a manual context compaction on an idle chat. @@ -3112,7 +3112,7 @@ func (c *ExperimentalClient) CompactChat(ctx context.Context, chatID uuid.UUID) return Chat{}, ReadBodyAsError(res) } var chat Chat - return chat, json.NewDecoder(res.Body).Decode(&chat) + return chat, ReadBodyAsJSON(res, &chat) } // ReconcileInvalidChatState recovers a chat stuck in an invalid @@ -3128,7 +3128,7 @@ func (c *ExperimentalClient) ReconcileInvalidChatState(ctx context.Context, chat return Chat{}, ReadBodyAsError(res) } var chat Chat - return chat, json.NewDecoder(res.Body).Decode(&chat) + return chat, ReadBodyAsJSON(res, &chat) } // RegenerateChatTitle requests the server to regenerate the chat's @@ -3143,7 +3143,7 @@ func (c *ExperimentalClient) RegenerateChatTitle(ctx context.Context, chatID uui return Chat{}, ReadBodyAsError(res) } var chat Chat - return chat, json.NewDecoder(res.Body).Decode(&chat) + return chat, ReadBodyAsJSON(res, &chat) } // ProposeChatTitleResponse is returned by the propose-title endpoint. @@ -3162,7 +3162,7 @@ func (c *ExperimentalClient) ProposeChatTitle(ctx context.Context, chatID uuid.U return ProposeChatTitleResponse{}, ReadBodyAsError(res) } var resp ProposeChatTitleResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // GetChatDiffContents returns resolved diff contents for a chat. @@ -3176,7 +3176,7 @@ func (c *ExperimentalClient) GetChatDiffContents(ctx context.Context, chatID uui return ChatDiffContents{}, ReadBodyAsError(res) } var diff ChatDiffContents - return diff, json.NewDecoder(res.Body).Decode(&diff) + return diff, ReadBodyAsJSON(res, &diff) } // UploadChatFile uploads a file for use in chat messages. @@ -3195,7 +3195,7 @@ func (c *ExperimentalClient) UploadChatFile(ctx context.Context, organizationID return UploadChatFileResponse{}, ReadBodyAsError(res) } var resp UploadChatFileResponse - return resp, json.NewDecoder(res.Body).Decode(&resp) + return resp, ReadBodyAsJSON(res, &resp) } // GetChatFile retrieves a previously uploaded chat file by ID. @@ -3246,5 +3246,5 @@ func (c *ExperimentalClient) GetChatsByWorkspace(ctx context.Context, workspaceI return nil, ReadBodyAsError(res) } var result map[uuid.UUID]uuid.UUID - return result, json.NewDecoder(res.Body).Decode(&result) + return result, ReadBodyAsJSON(res, &result) } diff --git a/codersdk/client.go b/codersdk/client.go index 3f42b9bf45..90e0fcf6ec 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -424,6 +424,49 @@ func ReadBodyAsError(res *http.Response) error { } defer res.Body.Close() + resp, err := io.ReadAll(res.Body) + if err != nil { + return xerrors.Errorf("read body: %w", err) + } + + if mimeErr := ExpectJSONMime(res); mimeErr != nil { + if len(resp) > 2048 { + resp = append(resp[:2048], []byte("...")...) + } + if len(resp) == 0 { + resp = []byte("no response body") + } + return newResponseError(res, Response{ + Message: mimeErr.Error(), + Detail: string(resp), + }) + } + + var m Response + err = json.NewDecoder(bytes.NewBuffer(resp)).Decode(&m) + if err != nil { + if errors.Is(err, io.EOF) { + return newResponseError(res, Response{ + Message: "empty response body", + }) + } + return xerrors.Errorf("decode body: %w", err) + } + if m.Message == "" { + if len(resp) > 1024 { + resp = append(resp[:1024], []byte("...")...) + } + m.Message = fmt.Sprintf("unexpected status code %d, response has no message", res.StatusCode) + m.Detail = string(resp) + } + + return newResponseError(res, m) +} + +// newResponseError wraps an API response in an *Error annotated with +// the status code, request method, and request URL from res. For 401 +// responses it also sets a helper message suggesting 'coder login'. +func newResponseError(res *http.Response, response Response) *Error { var requestMethod, requestURL string if res.Request != nil { requestMethod = res.Request.Method @@ -439,54 +482,8 @@ func ReadBodyAsError(res *http.Response) error { helpMessage = "Try logging in using 'coder login'." } - resp, err := io.ReadAll(res.Body) - if err != nil { - return xerrors.Errorf("read body: %w", err) - } - - if mimeErr := ExpectJSONMime(res); mimeErr != nil { - if len(resp) > 2048 { - resp = append(resp[:2048], []byte("...")...) - } - if len(resp) == 0 { - resp = []byte("no response body") - } - return &Error{ - statusCode: res.StatusCode, - method: requestMethod, - url: requestURL, - Response: Response{ - Message: mimeErr.Error(), - Detail: string(resp), - }, - Helper: helpMessage, - } - } - - var m Response - err = json.NewDecoder(bytes.NewBuffer(resp)).Decode(&m) - if err != nil { - if errors.Is(err, io.EOF) { - return &Error{ - statusCode: res.StatusCode, - Response: Response{ - Message: "empty response body", - }, - Helper: helpMessage, - } - } - return xerrors.Errorf("decode body: %w", err) - } - if m.Message == "" { - if len(resp) > 1024 { - resp = append(resp[:1024], []byte("...")...) - } - m.Message = fmt.Sprintf("unexpected status code %d, response has no message", res.StatusCode) - m.Detail = string(resp) - } - return &Error{ - Response: m, + Response: response, statusCode: res.StatusCode, method: requestMethod, url: requestURL, diff --git a/codersdk/client_internal_test.go b/codersdk/client_internal_test.go index fa6ae9ecb6..3dde597779 100644 --- a/codersdk/client_internal_test.go +++ b/codersdk/client_internal_test.go @@ -303,12 +303,18 @@ func Test_readBodyAsError(t *testing.T) { }, { name: "JSONNoBody", - req: nil, + req: httptest.NewRequest(http.MethodGet, exampleURL, nil), res: newResponse(http.StatusNotFound, jsonCT, ""), assert: func(t *testing.T, err error) { sdkErr := assertSDKError(t, err) assert.Contains(t, sdkErr.Response.Message, "empty response body") + + assert.Equal(t, http.MethodGet, sdkErr.method) + assert.ErrorContains(t, err, sdkErr.method) + + assert.Equal(t, exampleURL, sdkErr.url) + assert.ErrorContains(t, err, sdkErr.url) }, }, {