Skip to content

Commit

Permalink
*: improve logging and error handling for exits (#3347)
Browse files Browse the repository at this point in the history 
Add more logging and more detailed errors for exits. Given that the exits are fire and forget operation and not a long running process, we can afford more details without the issue of polluting with too much.

category: feature
ticket: #3136
  • Loading branch information
@KaloyanTanev
KaloyanTanev committed Nov 19, 2024
1 parent 48acae8 commit c4c3557
Show file tree
Hide file tree
Showing 14 changed files with 601 additions and 136 deletions.
51 changes: 41 additions & 10 deletions app/obolapi/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const (
func New(urlStr string, options ...func(*Client)) (Client, error) {
_, err := url.ParseRequestURI(urlStr) // check that urlStr is valid
if err != nil {
return Client{}, errors.Wrap(err, "could not parse Obol API URL")
return Client{}, errors.Wrap(err, "parse Obol API URL")
}

// always set a default timeout, even if no options are provided
Expand Down Expand Up @@ -63,7 +63,7 @@ func WithTimeout(timeout time.Duration) func(*Client) {
func (c Client) url() *url.URL {
baseURL, err := url.ParseRequestURI(c.baseURL)
if err != nil {
panic(errors.Wrap(err, "could not parse Obol API URL, this should never happen"))
panic(errors.Wrap(err, "parse Obol API URL, this should never happen"))
}

return baseURL
Expand All @@ -83,7 +83,7 @@ func (c Client) PublishLock(ctx context.Context, lock cluster.Lock) error {
ctx, cancel := context.WithTimeout(ctx, c.reqTimeout)
defer cancel()

err = httpPost(ctx, addr, b)
err = httpPost(ctx, addr, b, nil)
if err != nil {
return err
}
Expand All @@ -105,27 +105,58 @@ func launchpadURLPath(lock cluster.Lock) string {
return fmt.Sprintf(launchpadReturnPathFmt, lock.LockHash)
}

func httpPost(ctx context.Context, url *url.URL, b []byte) error {
req, err := http.NewRequestWithContext(ctx, http.MethodPost, url.String(), bytes.NewReader(b))
func httpPost(ctx context.Context, url *url.URL, body []byte, headers map[string]string) error {
req, err := http.NewRequestWithContext(ctx, http.MethodPost, url.String(), bytes.NewReader(body))
if err != nil {
return errors.Wrap(err, "new POST request with ctx")
}
req.Header.Add("Content-Type", "application/json")
for key, val := range headers {
req.Header.Set(key, val)
}

res, err := new(http.Client).Do(req)
if err != nil {
return errors.Wrap(err, "failed to call POST endpoint")
return errors.Wrap(err, "call POST endpoint")
}
defer res.Body.Close()

data, err := io.ReadAll(res.Body)
if res.StatusCode/100 != 2 {
data, err := io.ReadAll(res.Body)
if err != nil {
return errors.Wrap(err, "read POST response", z.Int("status", res.StatusCode))
}

return errors.New("http POST failed", z.Int("status", res.StatusCode), z.Str("body", string(data)))
}

return nil
}

func httpGet(ctx context.Context, url *url.URL, headers map[string]string) (io.ReadCloser, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url.String(), nil)
if err != nil {
return errors.Wrap(err, "failed to read POST response")
return nil, errors.Wrap(err, "new GET request with ctx")
}
req.Header.Add("Content-Type", "application/json")

for key, val := range headers {
req.Header.Set(key, val)
}

res, err := new(http.Client).Do(req)
if err != nil {
return nil, errors.Wrap(err, "call GET endpoint")
}

if res.StatusCode/100 != 2 {
return errors.New("post failed", z.Int("status", res.StatusCode), z.Str("body", string(data)))
data, err := io.ReadAll(res.Body)
if err != nil {
return nil, errors.Wrap(err, "read POST response", z.Int("status", res.StatusCode))
}

return nil, errors.New("http GET failed", z.Int("status", res.StatusCode), z.Str("body", string(data)))
}

return nil
return res.Body, nil
}
155 changes: 155 additions & 0 deletions app/obolapi/api_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
package obolapi

import (
"context"
"io"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"

Expand All @@ -21,3 +26,153 @@ func TestWithTimeout(t *testing.T) {
require.NoError(t, err)
require.Equal(t, timeout, oapi.reqTimeout)
}

func TestHttpPost(t *testing.T) {
tests := []struct {
name string
body []byte
headers map[string]string
server *httptest.Server
endpoint string
expectedError string
}{
{
name: "default scenario",
body: nil,
headers: nil,
endpoint: "/post-request",
server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, r.URL.Path, "/post-request")
require.Equal(t, r.Method, http.MethodPost)
require.Equal(t, r.Header.Get("Content-Type"), "application/json")
w.WriteHeader(http.StatusOK)
})),
expectedError: "",
},
{
name: "default scenario with body and headers",
body: []byte(`{"test_body_key": "test_body_value"}`),
headers: map[string]string{"test_header_key": "test_header_value"},
endpoint: "/post-request",
server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, r.URL.Path, "/post-request")
require.Equal(t, r.Method, http.MethodPost)
require.Equal(t, r.Header.Get("Content-Type"), "application/json")
require.Equal(t, r.Header.Get("test_header_key"), "test_header_value") //nolint:canonicalheader

data, err := io.ReadAll(r.Body)
require.NoError(t, err)
defer r.Body.Close()
require.Equal(t, string(data), `{"test_body_key": "test_body_value"}`)

w.WriteHeader(http.StatusOK)
_, err = w.Write([]byte(`"OK"`))
require.NoError(t, err)
})),
expectedError: "",
},
{
name: "status code not 2XX",
body: nil,
headers: nil,
endpoint: "/post-request",
server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, r.URL.Path, "/post-request")
require.Equal(t, r.Method, http.MethodPost)
require.Equal(t, r.Header.Get("Content-Type"), "application/json")

w.WriteHeader(http.StatusBadRequest)
_, err := w.Write([]byte(`"Bad Request response"`))
require.NoError(t, err)
})),
expectedError: "POST failed",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
testServerURL, err := url.ParseRequestURI(test.server.URL)
require.NoError(t, err)
err = httpPost(context.Background(), testServerURL.JoinPath(test.endpoint), test.body, test.headers)
if test.expectedError != "" {
require.Error(t, err)
require.ErrorContains(t, err, test.expectedError)
} else {
require.NoError(t, err)
}
})
}
}

func TestHttpGet(t *testing.T) {
tests := []struct {
name string
headers map[string]string
server *httptest.Server
endpoint string
expectedResp []byte
expectedError string
}{
{
name: "default scenario",
headers: nil,
endpoint: "/get-request",
server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, r.URL.Path, "/get-request")
require.Equal(t, r.Method, http.MethodGet)
require.Equal(t, r.Header.Get("Content-Type"), "application/json")
w.WriteHeader(http.StatusOK)
})),
expectedError: "",
},
{
name: "default scenario with headers",
headers: map[string]string{"test_header_key": "test_header_value"},
endpoint: "/get-request",
server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, r.URL.Path, "/get-request")
require.Equal(t, r.Method, http.MethodGet)
require.Equal(t, r.Header.Get("Content-Type"), "application/json")
require.Equal(t, r.Header.Get("test_header_key"), "test_header_value") //nolint:canonicalheader

w.WriteHeader(http.StatusOK)
_, err := w.Write([]byte(`"OK"`))
require.NoError(t, err)
})),
expectedResp: []byte(`"OK"`),
expectedError: "",
},
{
name: "status code not 2XX",
headers: nil,
endpoint: "/get-request",
server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, r.URL.Path, "/get-request")
require.Equal(t, r.Method, http.MethodGet)
require.Equal(t, r.Header.Get("Content-Type"), "application/json")

w.WriteHeader(http.StatusBadRequest)
_, err := w.Write([]byte(`"Bad Request response"`))
require.NoError(t, err)
})),
expectedResp: []byte(`"Bad Request response"`),
expectedError: "GET failed",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
testServerURL, err := url.ParseRequestURI(test.server.URL)
require.NoError(t, err)
respBody, err := httpGet(context.Background(), testServerURL.JoinPath(test.endpoint), test.headers)
if test.expectedError != "" {
require.Error(t, err)
require.ErrorContains(t, err, test.expectedError)
} else {
require.NoError(t, err)
defer respBody.Close()
resp, err := io.ReadAll(respBody)
require.NoError(t, err)
require.Equal(t, string(resp), string(test.expectedResp))
}
})
}
}
51 changes: 8 additions & 43 deletions app/obolapi/exit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@
package obolapi

import (
"bytes"
"context"
"encoding/hex"
"encoding/json"
"fmt"
"net/http"
"net/url"
"sort"
"strconv"
Expand Down Expand Up @@ -71,7 +69,7 @@ func (c Client) PostPartialExits(ctx context.Context, lockHash []byte, shareInde

u, err := url.ParseRequestURI(c.baseURL)
if err != nil {
return errors.Wrap(err, "bad obol api url")
return errors.Wrap(err, "bad Obol API url")
}

u.Path = path
Expand Down Expand Up @@ -107,24 +105,9 @@ func (c Client) PostPartialExits(ctx context.Context, lockHash []byte, shareInde
ctx, cancel := context.WithTimeout(ctx, c.reqTimeout)
defer cancel()

req, err := http.NewRequestWithContext(ctx, http.MethodPost, u.String(), bytes.NewReader(data))
err = httpPost(ctx, u, data, nil)
if err != nil {
return errors.Wrap(err, "http new post request")
}

req.Header.Set("Content-Type", "application/json")

resp, err := http.DefaultClient.Do(req)
if err != nil {
return errors.Wrap(err, "http post error")
}

defer func() {
_ = resp.Body.Close()
}()

if resp.StatusCode != http.StatusCreated {
return errors.New("http error", z.Int("status_code", resp.StatusCode))
return errors.Wrap(err, "http Obol API POST request")
}

return nil
Expand All @@ -142,19 +125,14 @@ func (c Client) GetFullExit(ctx context.Context, valPubkey string, lockHash []by

u, err := url.ParseRequestURI(c.baseURL)
if err != nil {
return ExitBlob{}, errors.Wrap(err, "bad obol api url")
return ExitBlob{}, errors.Wrap(err, "bad Obol API url")
}

u.Path = path

ctx, cancel := context.WithTimeout(ctx, c.reqTimeout)
defer cancel()

req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
if err != nil {
return ExitBlob{}, errors.Wrap(err, "http new get request")
}

exitAuthData := FullExitAuthBlob{
LockHash: lockHash,
ValidatorPubkey: valPubkeyBytes,
Expand All @@ -172,28 +150,15 @@ func (c Client) GetFullExit(ctx context.Context, valPubkey string, lockHash []by
return ExitBlob{}, errors.Wrap(err, "k1 sign")
}

req.Header.Set("Authorization", bearerString(lockHashSignature))
req.Header.Set("Content-Type", "application/json")

resp, err := http.DefaultClient.Do(req)
respBody, err := httpGet(ctx, u, map[string]string{"Authorization": bearerString(lockHashSignature)})
if err != nil {
return ExitBlob{}, errors.Wrap(err, "http get error")
}

if resp.StatusCode != http.StatusOK {
if resp.StatusCode == http.StatusNotFound {
return ExitBlob{}, ErrNoExit
}

return ExitBlob{}, errors.New("http error", z.Int("status_code", resp.StatusCode))
return ExitBlob{}, errors.Wrap(err, "http Obol API GET request")
}

defer func() {
_ = resp.Body.Close()
}()
defer respBody.Close()

var er FullExitResponse
if err := json.NewDecoder(resp.Body).Decode(&er); err != nil {
if err := json.NewDecoder(respBody).Decode(&er); err != nil {
return ExitBlob{}, errors.Wrap(err, "json unmarshal error")
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func New() *cobra.Command {
),
newExitCmd(
newListActiveValidatorsCmd(runListActiveValidatorsCmd),
newSubmitPartialExitCmd(runSignPartialExit),
newSignPartialExitCmd(runSignPartialExit),
newBcastFullExitCmd(runBcastFullExit),
newFetchExitCmd(runFetchExit),
),
Expand Down
2 changes: 1 addition & 1 deletion cmd/exit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func eth2Client(ctx context.Context, u []string, timeout time.Duration, forkVers
}

if _, err = cl.NodeVersion(ctx, &eth2api.NodeVersionOpts{}); err != nil {
return nil, errors.Wrap(err, "can't connect to beacon node")
return nil, errors.Wrap(err, "connect to beacon node")
}

return cl, nil
Expand Down
Loading

0 comments on commit c4c3557

Please sign in to comment.