Skip to content

Fixed error message when dialector fails to initialize#6509

Merged
jinzhu merged 2 commits intogo-gorm:masterfrom
RatajVaver:master
Aug 20, 2023
Merged

Fixed error message when dialector fails to initialize#6509
jinzhu merged 2 commits intogo-gorm:masterfrom
RatajVaver:master

Conversation

@RatajVaver
Copy link
Copy Markdown
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
Copy Markdown
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
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.

3 participants