-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
refactor: remove errors directive comparison #10502
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 | ||||
---|---|---|---|---|---|---|
|
@@ -300,7 +300,7 @@ func (c *ChannelBuilder) outputReadyFrames() error { | |||||
// When creating a frame from the ready compression data, the frame overhead | ||||||
// will be added to the total output size, so we can add it in the condition. | ||||||
for c.co.ReadyBytes()+derive.FrameV0OverHeadSize >= int(c.cfg.MaxFrameSize) { | ||||||
if err := c.outputFrame(); err == io.EOF { | ||||||
if err := c.outputFrame(); errors.Is(err, io.EOF) { | ||||||
return nil | ||||||
} else if err != nil { | ||||||
return err | ||||||
|
@@ -315,7 +315,7 @@ func (c *ChannelBuilder) closeAndOutputAllFrames() error { | |||||
} | ||||||
|
||||||
for { | ||||||
if err := c.outputFrame(); err == io.EOF { | ||||||
if err := c.outputFrame(); errors.Is(err, io.EOF) { | ||||||
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. Ensure proper error handling in - if err := c.outputFrame(); errors.Is(err, io.EOF) {
+ if err := c.outputFrame(); err != nil && !errors.Is(err, io.EOF) { Similar to the previous comment, this ensures that all errors are properly handled, not just Committable suggestion
Suggested change
|
||||||
return nil | ||||||
} else if err != nil { | ||||||
return err | ||||||
|
@@ -329,7 +329,7 @@ func (c *ChannelBuilder) closeAndOutputAllFrames() error { | |||||
func (c *ChannelBuilder) outputFrame() error { | ||||||
var buf bytes.Buffer | ||||||
fn, err := c.co.OutputFrame(&buf, c.cfg.MaxFrameSize) | ||||||
if err != io.EOF && err != nil { | ||||||
if !errors.Is(err, io.EOF) && err != nil { | ||||||
return fmt.Errorf("writing frame[%d]: %w", fn, err) | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -148,7 +148,7 @@ func (bq *BatchQueue) NextBatch(ctx context.Context, parent eth.L2BlockRef) (*Si | |||||
|
||||||
// Load more data into the batch queue | ||||||
outOfData := false | ||||||
if batch, err := bq.prev.NextBatch(ctx); err == io.EOF { | ||||||
if batch, err := bq.prev.NextBatch(ctx); errors.Is(err, io.EOF) { | ||||||
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. Ensure proper error handling in - if batch, err := bq.prev.NextBatch(ctx); errors.Is(err, io.EOF) {
+ if batch, err := bq.prev.NextBatch(ctx); err != nil && !errors.Is(err, io.EOF) { This change ensures that all errors are properly handled, not just Committable suggestion
Suggested change
|
||||||
outOfData = true | ||||||
} else if err != nil { | ||||||
return nil, false, err | ||||||
|
@@ -168,9 +168,9 @@ func (bq *BatchQueue) NextBatch(ctx context.Context, parent eth.L2BlockRef) (*Si | |||||
|
||||||
// Finally attempt to derive more batches | ||||||
batch, err := bq.deriveNextBatch(ctx, outOfData, parent) | ||||||
if err == io.EOF && outOfData { | ||||||
if errors.Is(err, io.EOF) && outOfData { | ||||||
return nil, false, io.EOF | ||||||
} else if err == io.EOF { | ||||||
} else if errors.Is(err, io.EOF) { | ||||||
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. Ensure proper error handling in - } else if errors.Is(err, io.EOF) {
+ } else if err != nil && !errors.Is(err, io.EOF) { This ensures that all errors are properly handled, not just Committable suggestion
Suggested change
|
||||||
return nil, false, NotEnoughData | ||||||
} else if err != nil { | ||||||
return nil, false, err | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -304,7 +304,7 @@ func (eq *EngineQueue) Step(ctx context.Context) error { | |||||||||||||||||||
// Trying unsafe payload should be done before safe attributes | ||||||||||||||||||||
// It allows the unsafe head can move forward while the long-range consolidation is in progress. | ||||||||||||||||||||
if eq.unsafePayloads.Len() > 0 { | ||||||||||||||||||||
if err := eq.tryNextUnsafePayload(ctx); err != io.EOF { | ||||||||||||||||||||
if err := eq.tryNextUnsafePayload(ctx); !errors.Is(err, io.EOF) { | ||||||||||||||||||||
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. Ensure proper error handling for non-EOF errors in - if err := eq.tryNextUnsafePayload(ctx); !errors.Is(err, io.EOF) {
+ if err := eq.tryNextUnsafePayload(ctx); err != nil && !errors.Is(err, io.EOF) {
return err
} The current implementation only handles the case where the error is not Committable suggestion
Suggested change
|
||||||||||||||||||||
return err | ||||||||||||||||||||
} | ||||||||||||||||||||
// EOF error means we can't process the next unsafe payload. Then we should process next safe attributes. | ||||||||||||||||||||
|
@@ -331,7 +331,7 @@ func (eq *EngineQueue) Step(ctx context.Context) error { | |||||||||||||||||||
if err := eq.tryFinalizePastL2Blocks(ctx); err != nil { | ||||||||||||||||||||
return err | ||||||||||||||||||||
} | ||||||||||||||||||||
if next, err := eq.prev.NextAttributes(ctx, eq.ec.PendingSafeL2Head()); err == io.EOF { | ||||||||||||||||||||
if next, err := eq.prev.NextAttributes(ctx, eq.ec.PendingSafeL2Head()); errors.Is(err, io.EOF) { | ||||||||||||||||||||
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. Handle potential - if next, err := eq.prev.NextAttributes(ctx, eq.ec.PendingSafeL2Head()); errors.Is(err, io.EOF) {
+ if next, err := eq.prev.NextAttributes(ctx, eq.ec.PendingSafeL2Head()); err != nil {
+ if errors.Is(err, io.EOF) {
return io.EOF
+ }
+ return err
} else if err != nil {
return err
} else { The original code does not handle other errors that might occur when fetching the next attributes. This change ensures that all errors are handled appropriately, not just Committable suggestion
Suggested change
|
||||||||||||||||||||
return io.EOF | ||||||||||||||||||||
} else if err != nil { | ||||||||||||||||||||
return err | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package derive | |
|
||
import ( | ||
"context" | ||
"errors" | ||
"io" | ||
"math/big" | ||
"math/rand" | ||
|
@@ -260,7 +261,7 @@ func TestPlasmaDataSource(t *testing.T) { | |
break | ||
} | ||
|
||
for data, err := src.Next(ctx); err != io.EOF; data, err = src.Next(ctx) { | ||
for data, err := src.Next(ctx); !errors.Is(err, io.EOF); data, err = src.Next(ctx) { | ||
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. Tip Codebase Verification The verification process has revealed that there are multiple instances across the codebase where error handling does not use the Locations needing attention:
These files contain direct comparisons of errors without using Analysis chainUse of Scripts ExecutedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that all instances of error handling in the project have been updated to use `errors.Is()`.
# Test: Search for direct error comparisons. Expect: No occurrences of direct error comparisons.
rg --type go '!errors.Is' --glob '*.go'
Length of output: 2982 |
||
logger.Info("yielding data") | ||
// check that each commitment is resolved | ||
require.NoError(t, err) | ||
|
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.
Ensure proper error handling in
outputReadyFrames
.The original code does not handle errors that are not
io.EOF
. This change ensures that all errors are properly handled.Committable suggestion