Skip to content

Commit 9de09f5

Browse files
Include the HTTP status code in jit error (#4361)
Co-authored-by: Dhawal Seth <dseth@linkedin.com>
1 parent 02aa70a commit 9de09f5

File tree

2 files changed

+40
-1
lines changed

2 files changed

+40
-1
lines changed

github/actions/client.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ func (c *Client) Identifier() string {
274274
func (c *Client) Do(req *http.Request) (*http.Response, error) {
275275
resp, err := c.Client.Do(req)
276276
if err != nil {
277+
// If we have a response even with an error, include the status code
278+
if resp != nil {
279+
return nil, fmt.Errorf("client request failed with status code %d: %w", resp.StatusCode, err)
280+
}
277281
return nil, fmt.Errorf("client request failed: %w", err)
278282
}
279283

@@ -856,7 +860,8 @@ func (c *Client) GenerateJitRunnerConfig(ctx context.Context, jitRunnerSetting *
856860

857861
resp, err := c.Do(req)
858862
if err != nil {
859-
return nil, fmt.Errorf("failed to issue the request: %w", err)
863+
// Include the URL and method in the error for better debugging
864+
return nil, fmt.Errorf("failed to issue the request %s %s: %w", req.Method, req.URL.String(), err)
860865
}
861866

862867
if resp.StatusCode != http.StatusOK {

github/actions/client_generate_jit_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package actions_test
22

33
import (
44
"context"
5+
"errors"
56
"net/http"
67
"testing"
78
"time"
@@ -58,4 +59,37 @@ func TestGenerateJitRunnerConfig(t *testing.T) {
5859
assert.NotNil(t, err)
5960
assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry)
6061
})
62+
63+
t.Run("Error includes HTTP method and URL when request fails", func(t *testing.T) {
64+
runnerSettings := &actions.RunnerScaleSetJitRunnerSetting{}
65+
66+
server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
67+
w.WriteHeader(http.StatusInternalServerError)
68+
}))
69+
70+
client, err := actions.NewClient(
71+
server.configURLForOrg("my-org"),
72+
auth,
73+
actions.WithRetryMax(0), // No retries to get immediate error
74+
actions.WithRetryWaitMax(1*time.Millisecond),
75+
)
76+
require.NoError(t, err)
77+
78+
_, err = client.GenerateJitRunnerConfig(ctx, runnerSettings, 1)
79+
require.NotNil(t, err)
80+
// Verify error message includes HTTP method and URL for better debugging
81+
errMsg := err.Error()
82+
assert.Contains(t, errMsg, "POST", "Error message should include HTTP method")
83+
assert.Contains(t, errMsg, "generatejitconfig", "Error message should include URL path")
84+
85+
// The error might be an ActionsError (if response was received) or a wrapped error (if Do() failed)
86+
// In either case, the error message should include request details
87+
var actionsErr *actions.ActionsError
88+
if errors.As(err, &actionsErr) {
89+
// If we got an ActionsError, verify the status code is included
90+
assert.Equal(t, http.StatusInternalServerError, actionsErr.StatusCode)
91+
}
92+
// If it's a wrapped error from Do(), the error message already includes the method and URL
93+
// which is what we're testing for
94+
})
6195
}

0 commit comments

Comments
 (0)