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

Avoid panics #389

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open

Avoid panics #389

wants to merge 19 commits into from

Conversation

pablodeymo
Copy link
Contributor

@pablodeymo pablodeymo commented Nov 1, 2024

Fixes # .

What Changed?

The functions BuildReadClients, BuildAll, NewZapLoggerByConfig, and AvsRegistryServiceChainCaller.GetOperatorsAvsStateAtBlock return errors instead of panicking.
The OperatorsInfoServiceInMemory no longer panics (errors are only logged).

Reviewer Checklist

  • Code is well-documented
  • Code adheres to Go naming conventions
  • Code deprecates any old functionality before removing it

@pablodeymo pablodeymo changed the title Change Errors fatal to return the error instead Avoid panics Nov 5, 2024
@pablodeymo pablodeymo marked this pull request as ready for review November 5, 2024 19:38
@pablodeymo pablodeymo added the v0.2 new dev version label Nov 12, 2024
metrics := NewMetrics(reg, "example", logger)
return client, NewGeometricTxnManager(client, wallet, logger, metrics, GeometricTxnManagerParams{}), nil
}

func ExampleGeometricTxManager() {
anvilC, err := testutils.StartAnvilContainer("")
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also avoid the use of panics in the examples as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@@ -76,11 +76,16 @@ func (ar *AvsRegistryServiceChainCaller) GetOperatorsAvsStateAtBlock(
}
numquorums := len(quorumNumbers)
if len(operatorsStakesInQuorums) != numquorums {
ar.logger.Fatal(
ar.logger.Error(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add the values of operatorsStakesInQuorums and numquorums in this log for debugging

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@@ -128,8 +123,9 @@ func (ops *OperatorsInfoServiceInMemory) startServiceInGoroutine(
queryC <-chan query,
wg *sync.WaitGroup,
opts Opts,
) {
go func() {
) chan<- error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more readable/and simplify error handling if we replaced the error channel with an error group here

Copy link
Contributor

Choose a reason for hiding this comment

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

Great idea! I added it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v0.2 new dev version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants