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

Fixed error message when dialector fails to initialize #6509

Merged
merged 2 commits into from Aug 20, 2023

Conversation

RatajVaver
Copy link
Contributor

@RatajVaver RatajVaver commented Aug 6, 2023

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Dialector initialize error leads to closing DB connection. However if DB connection is not even created, code inside gorm provides nil as an error even though db doesn't exist (is nil), so it tries to close what can't be closed and results to panic.
panic: runtime error: invalid memory address or nil pointer dereference

This fix now doesn't attempt to close non-existant database connection and instead continues, so the proper error is shown.

User Case Description

When I provide this example DSN (I am aware that this is wrong, it's a purpose to showcase this problem):
test:test@localhost/test?charset=utf8mb4&parseTime=True&loc=Local

To this piece of code:
db, err := gorm.Open(mysql.Open(dsn), &gorm.Config{})

I got:
panic: runtime error: invalid memory address or nil pointer dereference

With this fix I get proper error:
[error] failed to initialize database, got error default addr for network 'localhost' unknown

This error is now in err variable in my app, instead of it failing inside gorm.

Let's say we have a problem with DSN which leads to dialector initialize error. However DB connection is not created and for some reason line 184 error provides <nil> even though "db" doesn't exist.

Previously, this code leads to:
panic: runtime error: invalid memory address or nil pointer dereference

This fix now doesn't attempt to close non-existant database connection and instead continues, so the proper error is shown. In my case:
[error] failed to initialize database, got error default addr for network 'localhost' unknown
Fixed error message when dialector fails to initialize
Copy link
Member

@a631807682 a631807682 left a comment

Choose a reason for hiding this comment

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

Can you add some tests? related to #6373

@jinzhu jinzhu merged commit ac07543 into go-gorm:master Aug 20, 2023
25 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.

None yet

3 participants