- 
                Notifications
    You must be signed in to change notification settings 
- Fork 180
Refactor template request common methods #1250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 
 Valid issues, but not new here. They'll be fixed in another PR (#1249) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Variable Shadowing and Error Reporting Issues
The template variable shadows the imported template package in both template_request_build_v2.go and template_request_build.go, causing a compilation error. Additionally, telemetry reporting in both files logs the body parsing error (err) instead of the actual template build error (apiError.Err), leading to misleading error information.
packages/api/internal/handlers/template_request_build_v2.go#L78-L84
infra/packages/api/internal/handlers/template_request_build_v2.go
Lines 78 to 84 in f7d0b7c
| template, apiError := template.RegisterBuild(ctx, a.templateBuildsCache, a.db, a.sqlcDB, buildReq) | |
| if apiError != nil { | |
| a.sendAPIStoreError(c, apiError.Code, apiError.ClientMsg) | |
| telemetry.ReportCriticalError(ctx, "invalid request body", err) | |
| return | |
| } | 
packages/api/internal/handlers/template_request_build.go#L104-L110
infra/packages/api/internal/handlers/template_request_build.go
Lines 104 to 110 in f7d0b7c
| template, apiError := template.RegisterBuild(ctx, a.templateBuildsCache, a.db, a.sqlcDB, buildReq) | |
| if apiError != nil { | |
| a.sendAPIStoreError(c, apiError.Code, apiError.ClientMsg) | |
| telemetry.ReportCriticalError(ctx, "build template request failed", err) | |
| return nil | |
| } | 
Bug: Template Shadowing and Telemetry Error
The TemplateRequestBuild function introduces a compilation error where the template variable shadows the imported template package. It also incorrectly reports build failures to telemetry by using an unrelated error variable.
packages/api/internal/handlers/template_request_build.go#L104-L108
infra/packages/api/internal/handlers/template_request_build.go
Lines 104 to 108 in f7d0b7c
| template, apiError := template.RegisterBuild(ctx, a.templateBuildsCache, a.db, a.sqlcDB, buildReq) | |
| if apiError != nil { | |
| a.sendAPIStoreError(c, apiError.Code, apiError.ClientMsg) | |
| telemetry.ReportCriticalError(ctx, "build template request failed", err) | 
Refactor template request common methods (just moved around).