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

Enhance GetBridgeDBInstance: Add Error Handling for LevelDB Initialization #1201

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

vinayak0035
Copy link
Contributor

@vinayak0035 vinayak0035 commented Oct 29, 2024

Overview

Currently, the GetBridgeDBInstance function does not handle scenarios where bridgeDB may be nil. If the database fails to open for any reason, returning a nil object can lead to potential runtime panics or errors when the returned database instance is used. Adding error handling will help prevent these issues and improve the robustness of the code.

Changes Made

  • Added error handling within the sync.Once block to capture and return any errors encountered while opening the LevelDB database.

Code Changes

Before:

func GetBridgeDBInstance(filePath string) *leveldb.DB {
    bridgeDBOnce.Do(func() {
        bridgeDB, _ = leveldb.OpenFile(filePath, nil)
    })
    return bridgeDB
}

After:

func GetBridgeDBInstance(filePath string) *leveldb.DB {
	bridgeDBOnce.Do(func() {
		var err error
		bridgeDB, err = leveldb.OpenFile(filePath, nil)
		if err != nil {
			log.Fatalln("Error in Bor Opening Database", err.Error())
		}
	})
	return bridgeDB
}

Benefits

  • Enhanced reliability of the application.
  • Easier debugging by providing clear error messages.
  • Improved user experience by avoiding crashes due to nil references.

@pratikspatil024 pratikspatil024 requested a review from a team October 29, 2024 12:40
@pratikspatil024 pratikspatil024 linked an issue Oct 29, 2024 that may be closed by this pull request
4 tasks
@pratikspatil024 pratikspatil024 requested a review from a team October 29, 2024 12:50
@marcello33
Copy link
Contributor

Hey @vinayak0035, thanks for you contribution.
In your PR's description I see
Updated the GetBridgeDBInstance function signature to return an error alongside the database instance.
But the code does not reflect that. As it is currently, you're only logging an error in case levelDB is not able to open the file, but even with such change bridgeDB might be nil (hence the crash/panic would still occur).
Is that intended? Thank you.

@avalkov
Copy link
Contributor

avalkov commented Nov 14, 2024

I am ok with the fix, @vinayak0035 can you just update your description as your not updating the signature (which I agree is redundant because we open the db just once and in all other cases we would just return nil).

@vinayak0035
Copy link
Contributor Author

Hey @avalkov I updated the description , removed the signature.

@marcello33 marcello33 merged commit d90fa7a into maticnetwork:develop Nov 14, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Error Handling in GetBridgeDBInstance Function
4 participants