Skip to content
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

p2p receipts #11010

Merged
merged 33 commits into from
Jul 16, 2024
Merged
Changes from 1 commit
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
4 changes: 1 addition & 3 deletions erigon-lib/txpool/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (f *Fetch) receiveMessage(ctx context.Context, sentryClient sentry.SentryCl
var req *sentry.InboundMessage
for req, err = stream.Recv(); ; req, err = stream.Recv() {
if err != nil {
f.logger.Error("[txpool.receiveMessage]", "err", err)
f.logger.Debug("[txpool.receiveMessage]", "err", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

no reason to log ctx cancelation.
for example if node shutting-down by Ctrl+C - it's not an error, but expected behavior.
let's move under select or let's use errors.Is(context.Canceled)

Copy link
Member Author

Choose a reason for hiding this comment

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

return wrapped error instead of logging

Copy link
Collaborator

Choose a reason for hiding this comment

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

also: I understand why need log error which we not return. But if we return - then it's job of caller to log it (otherwise we may log same thing multiple times).
if need add context to error: can use fmt.Errorf("txpool.receiveMessage: %w", err)

Copy link
Member Author

Choose a reason for hiding this comment

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

return wrapped error instead of logging

select {
case <-f.ctx.Done():
return ctx.Err()
Expand All @@ -189,7 +189,6 @@ func (f *Fetch) receiveMessage(ctx context.Context, sentryClient sentry.SentryCl
f.logger.Warn("[txpool.receiveMessage]", "req nil")
return nil
}
f.logger.Info("[txpool.receiveMessage]", "req", req)
if err := f.handleInboundMessage(streamCtx, req, sentryClient); err != nil {
if grpcutil.IsRetryLater(err) || grpcutil.IsEndOfStream(err) {
time.Sleep(3 * time.Second)
Expand All @@ -200,7 +199,6 @@ func (f *Fetch) receiveMessage(ctx context.Context, sentryClient sentry.SentryCl
if f.wg != nil {
f.wg.Done()
}
f.logger.Info("[txpool.fetch] Handling incoming message", "msg", string(req.Data), "reqID", req.Id.String(), "err", err)
}
}

Expand Down
Loading