Skip to content

Commit

Permalink
fix: custom error wrapped around error and commitment meta
Browse files Browse the repository at this point in the history
  • Loading branch information
@hopeyen
hopeyen committed Sep 19, 2024
1 parent 0d37899 commit 89078cd
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 25 deletions.
53 changes: 39 additions & 14 deletions server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,23 @@ func NewServer(host string, port int, router store.IRouter, log log.Logger,
}

// WithMetrics is a middleware that records metrics for the route path.
func WithMetrics(handleFn func(http.ResponseWriter, *http.Request) (commitments.CommitmentMeta, error),
func WithMetrics(handleFn func(http.ResponseWriter, *http.Request) (commitments.CommitmentMeta, HandlerError),
m metrics.Metricer) func(http.ResponseWriter, *http.Request) error {
return func(w http.ResponseWriter, r *http.Request) error {
recordDur := m.RecordRPCServerRequest(r.Method)

meta, err := handleFn(w, r)
if err.Err != nil {
if err.Meta != nil {
recordDur(w.Header().Get("status"), string(err.Meta.Mode), string(err.Meta.CertVersion))
} else {
recordDur(w.Header().Get("status"), string("NoCommitmentModeSet"), string("NoCertVersionSet"))
}
return err.Err
}
// we assume that every route will set the status header
recordDur(w.Header().Get("status"), string(meta.Mode), string(meta.CertVersion))
return err
return nil
}
}

Expand Down Expand Up @@ -150,19 +158,19 @@ func (svr *Server) Health(w http.ResponseWriter, _ *http.Request) error {
// Note: even when an error is returned, the commitment meta is still returned,
// because it is needed for metrics (see the WithMetrics middleware).
// TODO: we should change this behavior and instead use a custom error that contains the commitment meta.
func (svr *Server) HandleGet(w http.ResponseWriter, r *http.Request) (commitments.CommitmentMeta, error) {
func (svr *Server) HandleGet(w http.ResponseWriter, r *http.Request) (commitments.CommitmentMeta, HandlerError) {
meta, err := ReadCommitmentMeta(r)
if err != nil {
err = fmt.Errorf("invalid commitment mode: %w", err)
svr.WriteBadRequest(w, err)
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}
key := path.Base(r.URL.Path)
comm, err := commitments.StringToDecodedCommitment(key, meta.Mode)
if err != nil {
err = fmt.Errorf("failed to decode commitment from key %v (commitment mode %v): %w", key, meta.Mode, err)
svr.WriteBadRequest(w, err)
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}

input, err := svr.router.Get(r.Context(), comm, meta.Mode)
Expand All @@ -173,30 +181,30 @@ func (svr *Server) HandleGet(w http.ResponseWriter, r *http.Request) (commitment
} else {
svr.WriteInternalError(w, err)
}
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}

svr.WriteResponse(w, input)
return meta, nil
return meta, HandlerError{}
}

// HandlePut handles the PUT request for commitments.
// Note: even when an error is returned, the commitment meta is still returned,
// because it is needed for metrics (see the WithMetrics middleware).
// TODO: we should change this behavior and instead use a custom error that contains the commitment meta.
func (svr *Server) HandlePut(w http.ResponseWriter, r *http.Request) (commitments.CommitmentMeta, error) {
func (svr *Server) HandlePut(w http.ResponseWriter, r *http.Request) (commitments.CommitmentMeta, HandlerError) {
meta, err := ReadCommitmentMeta(r)
if err != nil {
err = fmt.Errorf("invalid commitment mode: %w", err)
svr.WriteBadRequest(w, err)
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}

input, err := io.ReadAll(r.Body)
if err != nil {
err = fmt.Errorf("failed to read request body: %w", err)
svr.WriteBadRequest(w, err)
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}

key := path.Base(r.URL.Path)
Expand All @@ -207,28 +215,28 @@ func (svr *Server) HandlePut(w http.ResponseWriter, r *http.Request) (commitment
if err != nil {
err = fmt.Errorf("failed to decode commitment from key %v (commitment mode %v): %w", key, meta.Mode, err)
svr.WriteBadRequest(w, err)
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}
}

commitment, err := svr.router.Put(r.Context(), meta.Mode, comm, input)
if err != nil {
err = fmt.Errorf("put request failed with commitment %v (commitment mode %v): %w", comm, meta.Mode, err)
svr.WriteInternalError(w, err)
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}

responseCommit, err := commitments.EncodeCommitment(commitment, meta.Mode)
if err != nil {
err = fmt.Errorf("failed to encode commitment %v (commitment mode %v): %w", commitment, meta.Mode, err)
svr.WriteInternalError(w, err)
return meta, err
return commitments.CommitmentMeta{}, NewHandlerError(err, &meta)
}

svr.log.Info(fmt.Sprintf("write commitment: %x\n", comm))
// write out encoded commitment
svr.WriteResponse(w, responseCommit)
return meta, nil
return meta, HandlerError{}
}

func (svr *Server) WriteResponse(w http.ResponseWriter, data []byte) {
Expand Down Expand Up @@ -363,3 +371,20 @@ func (svr *Server) GetStoreStats(bt store.BackendType) (*store.Stats, error) {

return nil, fmt.Errorf("store not found")
}

type HandlerError struct {
Err error
Meta *commitments.CommitmentMeta
}

func (e *HandlerError) Error() string {
return fmt.Sprintf("HandlerError: %s", e.Err.Error())
}

// Helper function to create a new HandlerError
func NewHandlerError(err error, meta *commitments.CommitmentMeta) HandlerError {
return HandlerError{
Err: err,
Meta: meta,
}
}
21 changes: 10 additions & 11 deletions server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestGetHandler(t *testing.T) {
expectedCode: http.StatusInternalServerError,
expectedBody: "",
expectError: true,
expectedCommitmentMeta: commitments.CommitmentMeta{Mode: commitments.OptimismGeneric, CertVersion: 0},
expectedCommitmentMeta: commitments.CommitmentMeta{},
},
{
name: "Success - OP Keccak256",
Expand All @@ -107,7 +107,7 @@ func TestGetHandler(t *testing.T) {
expectedCode: http.StatusInternalServerError,
expectedBody: "",
expectError: true,
expectedCommitmentMeta: commitments.CommitmentMeta{Mode: commitments.OptimismAltDA, CertVersion: 0},
expectedCommitmentMeta: commitments.CommitmentMeta{},
},
{
name: "Success - OP Alt-DA",
Expand All @@ -131,9 +131,9 @@ func TestGetHandler(t *testing.T) {

meta, err := server.HandleGet(rec, req)
if tt.expectError {
require.Error(t, err)
require.Error(t, err.Err)
} else {
require.NoError(t, err)
require.NoError(t, err.Err)
}

require.Equal(t, tt.expectedCode, rec.Code)
Expand Down Expand Up @@ -217,11 +217,10 @@ func TestPutHandler(t *testing.T) {
mockBehavior: func() {
mockRouter.EXPECT().Put(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("internal error"))
},
expectedCode: http.StatusInternalServerError,
expectedBody: "",
expectError: true,
// certification version is the third byte of the body, in this case it's "m"
expectedCommitmentMeta: commitments.CommitmentMeta{Mode: commitments.OptimismAltDA, CertVersion: 0},
expectedCode: http.StatusInternalServerError,
expectedBody: "",
expectError: true,
expectedCommitmentMeta: commitments.CommitmentMeta{},
},
{
name: "Success OP Mode Alt-DA",
Expand Down Expand Up @@ -270,9 +269,9 @@ func TestPutHandler(t *testing.T) {

meta, err := server.HandlePut(rec, req)
if tt.expectError {
require.Error(t, err)
require.Error(t, err.Err)
} else {
require.NoError(t, err)
require.NoError(t, err.Err)
}
require.Equal(t, tt.expectedCode, rec.Code)
if !tt.expectError {
Expand Down

0 comments on commit 89078cd

Please sign in to comment.