Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions module/pipeline_step_http_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,13 +470,15 @@ func (s *HTTPCallStep) Execute(ctx context.Context, pc *PipelineContext) (*StepR
return nil, err
}

start := time.Now()
resp, err := s.httpClient.Do(req) //nolint:gosec // G107: URL is user-configured
if err != nil {
return nil, fmt.Errorf("http_call step %q: request failed: %w", s.name, err)
}
defer resp.Body.Close()

respBody, err := io.ReadAll(resp.Body)
elapsedMS := time.Since(start).Milliseconds()
if err != nil {
return nil, fmt.Errorf("http_call step %q: failed to read response: %w", s.name, err)
}
Expand Down Expand Up @@ -511,18 +513,21 @@ func (s *HTTPCallStep) Execute(ctx context.Context, pc *PipelineContext) (*StepR
return nil, buildErr
}

retryStart := time.Now()
retryResp, doErr := s.httpClient.Do(retryReq) //nolint:gosec // G107: URL is user-configured
if doErr != nil {
return nil, fmt.Errorf("http_call step %q: retry request failed: %w", s.name, doErr)
}
defer retryResp.Body.Close()

respBody, err = io.ReadAll(retryResp.Body)
retryElapsedMS := time.Since(retryStart).Milliseconds()
if err != nil {
return nil, fmt.Errorf("http_call step %q: failed to read retry response: %w", s.name, err)
}

output := parseHTTPResponse(retryResp, respBody)
output["elapsed_ms"] = retryElapsedMS
if instanceURL := s.oauthEntry.getInstanceURL(); instanceURL != "" {
Comment on lines 523 to 531
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retry-on-401 path now sets elapsed_ms, but there’s no assertion covering this behavior. Since there’s already a 401 refresh test covering the retry flow, consider extending it (or adding a dedicated test) to assert elapsed_ms is present and non-negative on the retry output as well, so this key doesn’t regress on the refresh path.

Copilot generated this review using guidance from organization custom instructions.
output["instance_url"] = instanceURL
}
Expand All @@ -533,6 +538,7 @@ func (s *HTTPCallStep) Execute(ctx context.Context, pc *PipelineContext) (*StepR
}

output := parseHTTPResponse(resp, respBody)
output["elapsed_ms"] = elapsedMS
if s.auth != nil {
if instanceURL := s.oauthEntry.getInstanceURL(); instanceURL != "" {
output["instance_url"] = instanceURL
Expand Down
39 changes: 39 additions & 0 deletions module/pipeline_step_http_call_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ func TestHTTPCallStep_OAuth2_Retry401(t *testing.T) {
if result.Output["status_code"] != http.StatusOK {
t.Errorf("expected 200 after retry, got %v", result.Output["status_code"])
}
retryElapsedMS, ok := result.Output["elapsed_ms"].(int64)
if !ok {
t.Fatalf("expected elapsed_ms to be int64 on retry path, got %T (%v)", result.Output["elapsed_ms"], result.Output["elapsed_ms"])
}
if retryElapsedMS < 0 {
t.Errorf("expected elapsed_ms >= 0 on retry path, got %d", retryElapsedMS)
}
if atomic.LoadInt32(&tokenRequests) != 2 {
t.Errorf("expected 2 token requests, got %d", atomic.LoadInt32(&tokenRequests))
}
Expand Down Expand Up @@ -1076,3 +1083,35 @@ func TestHTTPCallStep_BodyFrom_NilValue(t *testing.T) {
t.Errorf("expected empty body for nil body_from, got %q", string(gotBody))
}
}

func TestHTTPCallStep_ElapsedMS(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{}`))
}))
defer srv.Close()

factory := NewHTTPCallStepFactory()
step, err := factory("elapsed-test", map[string]any{
"url": srv.URL,
"method": "GET",
}, nil)
if err != nil {
t.Fatalf("factory error: %v", err)
}
step.(*HTTPCallStep).httpClient = srv.Client()

pc := NewPipelineContext(nil, nil)
result, err := step.Execute(context.Background(), pc)
if err != nil {
t.Fatalf("execute error: %v", err)
}

elapsedMS, ok := result.Output["elapsed_ms"].(int64)
if !ok {
t.Fatalf("expected elapsed_ms to be int64, got %T (%v)", result.Output["elapsed_ms"], result.Output["elapsed_ms"])
}
if elapsedMS < 0 {
t.Errorf("expected elapsed_ms >= 0, got %d", elapsedMS)
}
}
4 changes: 3 additions & 1 deletion schema/step_schema_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,11 @@ func (r *StepSchemaRegistry) registerBuiltins() {
{Key: "auth", Type: FieldTypeMap, Description: "Authentication config (type, token, client_id, client_secret, token_url for OAuth2)"},
},
Outputs: []StepOutputDef{
{Key: "status", Type: "number", Description: "HTTP response status code"},
{Key: "status_code", Type: "number", Description: "HTTP response status code (e.g. 200)"},
{Key: "status", Type: "string", Description: "HTTP response status text (e.g. \"200 OK\")"},
{Key: "body", Type: "any", Description: "Response body (parsed as JSON if Content-Type is application/json)"},
{Key: "headers", Type: "map", Description: "Response headers"},
{Key: "elapsed_ms", Type: "number", Description: "Request duration in milliseconds (wall-clock time from send to response fully read)"},
},
})

Expand Down
2 changes: 1 addition & 1 deletion schema/step_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func TestInferStepOutputs_Fallback(t *testing.T) {
for _, o := range outputs {
keys[o.Key] = true
}
if !keys["status"] || !keys["body"] || !keys["headers"] {
if !keys["status_code"] || !keys["body"] || !keys["headers"] || !keys["elapsed_ms"] {
t.Errorf("expected static outputs for step.http_call, got %v", outputs)
}
}
Expand Down
Loading