-
Notifications
You must be signed in to change notification settings - Fork 47
fix: error marshalling in processing reports #855
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -10,6 +10,16 @@ import ( | |||||||||||||||
|
||||||||||||||||
"github.com/filecoin-project/go-address" | ||||||||||||||||
"github.com/filecoin-project/go-hamt-ipld/v3" | ||||||||||||||||
"github.com/filecoin-project/lotus/chain/state" | ||||||||||||||||
"github.com/filecoin-project/lotus/chain/types" | ||||||||||||||||
"github.com/ipfs/go-cid" | ||||||||||||||||
logging "github.com/ipfs/go-log/v2" | ||||||||||||||||
"go.opencensus.io/stats" | ||||||||||||||||
"go.opencensus.io/tag" | ||||||||||||||||
"go.opentelemetry.io/otel" | ||||||||||||||||
"go.opentelemetry.io/otel/attribute" | ||||||||||||||||
"golang.org/x/xerrors" | ||||||||||||||||
|
||||||||||||||||
"github.com/filecoin-project/lily/chain/actors/adt" | ||||||||||||||||
init_ "github.com/filecoin-project/lily/chain/actors/builtin/init" | ||||||||||||||||
"github.com/filecoin-project/lily/chain/actors/builtin/market" | ||||||||||||||||
|
@@ -29,15 +39,6 @@ import ( | |||||||||||||||
"github.com/filecoin-project/lily/tasks/messageexecutions" | ||||||||||||||||
"github.com/filecoin-project/lily/tasks/messages" | ||||||||||||||||
"github.com/filecoin-project/lily/tasks/msapprovals" | ||||||||||||||||
"github.com/filecoin-project/lotus/chain/state" | ||||||||||||||||
"github.com/filecoin-project/lotus/chain/types" | ||||||||||||||||
"github.com/ipfs/go-cid" | ||||||||||||||||
logging "github.com/ipfs/go-log/v2" | ||||||||||||||||
"go.opencensus.io/stats" | ||||||||||||||||
"go.opencensus.io/tag" | ||||||||||||||||
"go.opentelemetry.io/otel" | ||||||||||||||||
"go.opentelemetry.io/otel/attribute" | ||||||||||||||||
"golang.org/x/xerrors" | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
const ( | ||||||||||||||||
|
@@ -434,7 +435,15 @@ func (t *TipSetIndexer) TipSet(ctx context.Context, ts *types.TipSet) error { | |||||||||||||||
res.Report[idx].StartedAt = res.StartedAt | ||||||||||||||||
res.Report[idx].CompletedAt = res.CompletedAt | ||||||||||||||||
|
||||||||||||||||
if res.Report[idx].ErrorsDetected != nil { | ||||||||||||||||
if err := res.Report[idx].ErrorsDetected; err != nil { | ||||||||||||||||
// because error is just an interface it may hold a value of any concrete type that implements it, and if | ||||||||||||||||
// said type has unexported fields json marshaling will fail when persisting. | ||||||||||||||||
e, ok := err.(error) | ||||||||||||||||
if ok { | ||||||||||||||||
res.Report[idx].ErrorsDetected = &struct { | ||||||||||||||||
Error string | ||||||||||||||||
}{Error: e.Error()} | ||||||||||||||||
} | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth putting a placeholder for Error if it can't be extracted? Something like "invalid/empty error detail" or similar? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Humm we could probably do that, but it would mean enforcing everything that writes to the struct filed being an error since it can contain types that do not implement the error interface: Lines 164 to 170 in fe9a838
I'm not sure what the best way to enforce that would be, I'm not sure if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm going to get this merged, but please continue here if anything comes to mind. |
||||||||||||||||
res.Report[idx].Status = visormodel.ProcessingStatusError | ||||||||||||||||
} else if res.Report[idx].StatusInformation != "" { | ||||||||||||||||
res.Report[idx].Status = visormodel.ProcessingStatusInfo | ||||||||||||||||
|
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.
Nothing changing here, just updated my editor.