Skip to content

fix: fix context propagation in UseDB method #1278

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

Merged
merged 2 commits into from
Mar 25, 2025

Conversation

hlts2
Copy link
Contributor

@hlts2 hlts2 commented Mar 22, 2025

  • Do only one thing
  • Non breaking API changes
  • Tested (CI and Local)

What did this pull request do?

First and foremost, Thank you for creating this great project.

WHAT

The issue where the context was always set to context.Background() in the UseDB method has been fixed. Previously, when performing database operations, the original context (including cancellation and timeout) was ignored, and a new context.Background() was always used. After the fix, the method now uses db.Statement.Context to propagate the existing context. If no context is set, it defaults to context.Background().

In the following example, even though the context (ctx) was passed from the higher layer, the original context was not properly propagated in the previous implementation:

t := query.Use(r.db.WithContext(ctx)).Tenant
tenant, err := t.Where(t.ID.Eq(id)).First()

WHY

By always setting the context to context.Background(), the original context's cancellation and timeout settings were not applied to the database queries, leading to potential issues with request timeouts or cancellations not being respected. This fix ensures that the cancellation and timeout of the context are properly propagated, allowing database operations to be influenced by the context. This will ensure that when a context is passed from a higher layer, like in the example above, it will correctly reflect cancellations or timeouts.

User Case Description

@hlts2 hlts2 changed the title fix: context propagation in useDB method Fix Context Propagation in UseDB Method Mar 22, 2025
@hlts2
Copy link
Contributor Author

hlts2 commented Mar 22, 2025

Hi @tr1v3r @qqxhb
Could you please review this PR? 🙇

@qqxhb
Copy link
Member

qqxhb commented Mar 24, 2025

The PR is fine. But it feels like there is a problem with the usage.
UseDB should only be called during initialization, and when used, it should bequery.User.WithContetx(ctx).xx
You can refer to this demo.
https://github.com/go-gorm/gendemo/blob/master/biz/dal/init.go

@hlts2
Copy link
Contributor Author

hlts2 commented Mar 24, 2025

@qqxhb cc: @tr1v3r

Thank you for your response, and I appreciate the detailed explanation.
I understand now. Can this PR be merged? 🤔 (or Should I close it?)

What I wanted to say is that, as described in the GORM documentation, I was trying Continuous Session Mode using gen.
Reference: Continuous Session Mode

When implementing it via gen, I thought it would work like this:

tx := query.Use(r.db.WithContext(ctx))

t := tx.Tenant
tenant, err := t.Where(t.ID.Eq(id)).First()
....

tx.〇〇
...

However, I encountered an issue where the Context was not properly propagated.

As I checked the code generated by gen, I eventually reached the following part.
I found that UseDB is used internally, which means that even if I pass a db instance with Context, the existing implementation effectively ignores it.
That’s why I created this PR 🙏

  • gen.go
// Code generated by gorm.io/gen. DO NOT EDIT.

package query

func Use(db *gorm.DB, opts ...gen.DOOption) *Query {
    return &Query{
        db:              db,
        ...
        Tenant:          newTenant(db, opts...),
        ...
    }
}
  • tenant.gen.go
func newTenant(db *gorm.DB, opts ...gen.DOOption) tenant {
    _tenant := tenant{}

    _tenant.tenantDo.UseDB(db, opts...) // <------------------ `UseDB` is called here
    _tenant.tenantDo.UseModel(&model.Tenant{})

    tableName := _tenant.tenantDo.TableName()
    .....

    return _tenant
}

@tr1v3r tr1v3r changed the title Fix Context Propagation in UseDB Method fix: fix context propagation in UseDB method Mar 25, 2025
@tr1v3r tr1v3r merged commit 9eb6c60 into go-gorm:master Mar 25, 2025
13 checks passed
@hlts2 hlts2 deleted the fix/db-context branch March 25, 2025 04:11
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