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

Don't AddError for Rollback on ErrTxDone #2434

Conversation

j16r
Copy link
Contributor

@j16r j16r commented May 1, 2019

When db.Rollback fails, check the error code. If it matches sql.ErrTxDone, we won't treat it as an error. This is expected and normal behavior of using Rollback with defer.

e.g:

tx := db.Begin()
defer tx.Rollback()
...
return tx.Commit().Error

See more discussion here: go-gormigrate/gormigrate#36

@j16r j16r force-pushed the feature/on_defer_rollback_should_not_print_error_message branch from f075ae0 to 33dbd12 Compare May 2, 2019 17:45
@codecov-io
Copy link

codecov-io commented May 5, 2019

Codecov Report

Merging #2434 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2434      +/-   ##
=========================================
- Coverage   80.15%   80.1%   -0.06%     
=========================================
  Files          24      24              
  Lines        3432    3433       +1     
=========================================
- Hits         2751    2750       -1     
- Misses        578     580       +2     
  Partials      103     103
Impacted Files Coverage Δ
main.go 82.58% <0%> (-0.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b715619...9f02bee. Read the comment docs.

@j16r j16r force-pushed the feature/on_defer_rollback_should_not_print_error_message branch from c17255f to 9f02bee Compare May 8, 2019 15:19
@j16r
Copy link
Contributor Author

j16r commented May 10, 2019

@javie2007 what do you think it would take to get this merged? The coverage could be a little tricky, not sure how to trigger an appropriate error in Rollback.

@andreynering
Copy link
Contributor

Ping @jinzhu @emirb

@jinzhu jinzhu merged commit ea12400 into go-gorm:master Jun 10, 2019
@j16r j16r deleted the feature/on_defer_rollback_should_not_print_error_message branch June 10, 2019 18:37
blefevre pushed a commit to blefevre/gorm that referenced this pull request Feb 17, 2020
blefevre pushed a commit to blefevre/gorm that referenced this pull request Feb 17, 2020
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.

5 participants