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

[ISSUE] reflect: NumField of non-struct type #18

Open
vzool opened this issue Sep 24, 2019 · 10 comments
Open

[ISSUE] reflect: NumField of non-struct type #18

vzool opened this issue Sep 24, 2019 · 10 comments

Comments

@vzool
Copy link

vzool commented Sep 24, 2019

Hi,

I have to use your package with the package hide.ID and I have an error with Update and Delete. Here is the full error report:
I use SQLite driver

2019/09/24 14:39:23 [Recovery] 2019/09/24 - 14:39:23 panic recovered:
PUT /api/v1/country/lk4KbkPpG HTTP/1.1
Host: localhost:8080
Accept: application/json, text/plain, */*
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9,ar;q=0.8
Authorization: *
Connection: keep-alive
Content-Length: 29
Content-Type: application/json;charset=UTF-8
Origin: http://localhost:3000
Referer: http://localhost:3000/
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-site
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36

reflect: NumField of non-struct type
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x4333d8)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:884 (0x989542)
	(*Scope).callCallbacks.func1: panic(err)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:522 (0x45ebda)
	call32: CALLFNcall32, 32)
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x4333d8)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/usr/lib64/go/1.11/src/reflect/type.go:984 (0x4b4b4d)
	(*rtype).NumField: panic("reflect: NumField of non-struct type")
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/util.go:99 (0x997b32)
	getLoggableFieldNames: for i := 0; i < t.NumField(); i++ {
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:141 (0x9954cb)
	computeUpdateDiff: names := getLoggableFieldNames(old)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:88 (0x994d81)
	addUpdateRecord: diff := computeUpdateDiff(scope)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:70 (0x994a27)
	(*Plugin).addUpdated: addUpdateRecord(scope, p.opts)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/plugin.go:25 (0x998543)
	(*Plugin).addUpdated-fm: callback.Update().After("gorm:after_update").Register("loggable:update", p.addUpdated)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:888 (0x974b47)
	(*Scope).callCallbacks: (*f)(scope)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/main.go:467 (0x9648bf)
	(*DB).Save: newDB := scope.callCallbacks(s.parent.callbacks.updates).db
/home/vzool/Workspace/seren/webapp/app/http/controllers/country.go:210 (0xbd0b81)
	CountryUpdate: if err := db.Save(&data).Error; err != nil {
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/auth.go:55 (0xbf5492)
	Auth.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/hideID.go:32 (0xbf5ef1)
	HideID.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/cors.go:26 (0xbf5b72)
	CORS.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-contrib/gzip@v0.0.1/gzip.go:47 (0xbf155e)
	Gzip.func2: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/secure.go:42 (0xbf61bf)
	Secure.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/recovery.go:83 (0x8e7c19)
	RecoveryWithWriter.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/logger.go:240 (0x8e6d80)
	LoggerWithConfig.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:389 (0x8de0c4)
	(*Engine).handleHTTPRequest: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:351 (0x8dd8f1)
	(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/usr/lib64/go/1.11/src/net/http/server.go:2741 (0x70760a)
	serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/lib64/go/1.11/src/net/http/server.go:1847 (0x703855)
	(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:1333 (0x4608f0)
	goexit: BYTE	$0x90	// NOP

Any advice could help me out? Thanks

@vzool
Copy link
Author

vzool commented Sep 24, 2019

After some investigation I found that when I enabled loggable.ComputeDiff() in loggable.Register() the issue occurred and when I removed the error will go. So the main issue should be within loggable.ComputeDiff() function.

@sas1024
Copy link
Owner

sas1024 commented Sep 24, 2019

Hello @vzool.
Can you show me the structure of data variable somewhere in /home/vzool/Workspace/seren/webapp/app/http/controllers/country.go:210 ?

@vzool
Copy link
Author

vzool commented Sep 25, 2019

Hi @sas1024, data is an instance of Country struct so here it is:

// BaseModel model
type BaseModel struct {
	ID hide.ID `gorm:"primary_key" json:"id,omitempty"`
}

// TrackingModel model
type TrackingModel struct {
	CreatedAt *time.Time `json:"created_at,omitempty" gorm:"NOT NULL"`
	UpdatedAt *time.Time `json:"updated_at,omitempty" gorm:"DEFAULT NULL"`
	DeletedAt *time.Time `sql:"index" json:"deleted_at,omitempty" gorm:"DEFAULT NULL"`

	CreatedBy hide.ID `json:"created_by,omitempty" gorm:"NOT NULL"`
	UpdatedBy hide.ID `json:"updated_by,omitempty" gorm:"DEFAULT NULL"`
	DeletedBy hide.ID `json:"deleted_by,omitempty" gorm:"DEFAULT NULL"`

	loggable.LoggableModel
}

// Country model
type Country struct {
	BaseModel

	Name string `json:"name,omitempty" form:"name" valid:"required" gorm:"unique_index;NOT NULL" gorm-loggable:"true"`
	Code int    `json:"code,omitempty" form:"code" gorm:"unique_index;NOT NULL" gorm-loggable:"true"`

	TrackingModel
}

@vzool
Copy link
Author

vzool commented Sep 25, 2019

After extra investigation I think that this line below inside callbacks/computeUpdateDiff():

old := im.get(scope.Value, scope.PrimaryKeyValue())

Does not return the old record to make Diff operation with new one.

@sas1024
Copy link
Owner

sas1024 commented Sep 25, 2019

I think you need to move loggable.LoggableModel from TrackingModel to Country.

@vzool
Copy link
Author

vzool commented Sep 25, 2019

This let me pass the error and fall into a new one:

reflect: call of reflect.Value.Field on ptr Value

And I can't share the complete logs because I changed a lot of things which become messy.

In fact, I did managed to pass first error with the following update:

func getLoggableFieldNames(value interface{}) []string {
	var names []string

	t := pointerType(reflect.TypeOf(value))

	for i := 0; i < t.NumField(); i++ {
		field := t.Field(i)
		value, ok := field.Tag.Lookup(loggableTag)
		if !ok || value != "true" {
			continue
		}

		names = append(names, field.Name)
	}

	fmt.Println("(Names): ", names)

	return names
}

// pointerType is made to chase for value through all the way following,
// leading pointers until reach the deep final value which is not a pointer
func pointerType(t reflect.Type) reflect.Type {
	for {
		if t.Kind() != reflect.Ptr {
			return t
		}
		t = t.Elem()
	}
}

So, I think it doesn't matter any more if I shift loggable.LoggableModel into upper level or leave it in the TrackingModel. Because the pointerType function will affects over all code and it behaves now more dynamic.

@vzool
Copy link
Author

vzool commented Sep 25, 2019

Okey I did reset the code and just add pointerType function with its call inside callbacks/getLoggableFieldNames() and produce the issue which gives me this:

2019/09/25 17:01:39 [Recovery] 2019/09/25 - 17:01:39 panic recovered:
PUT /api/v1/country/e0YPEGdBM HTTP/1.1
Host: localhost:8080
Accept: */*
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9,ar;q=0.8
Authorization: *
Cache-Control: no-cache
Connection: keep-alive
Content-Length: 45
Content-Type: text/plain;charset=UTF-8
Origin: chrome-extension://fhbjgbiflinjbdggehcddcbncdddomop
Postman-Token: 8a460257-d870-b7f8-5ac6-ae1159a0c9ff
Sec-Fetch-Mode: cors
Sec-Fetch-Site: cross-site
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36


reflect: call of reflect.Value.FieldByName on ptr Value
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x433358)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:884 (0x985422)
	(*Scope).callCallbacks.func1: panic(err)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:522 (0x45eb0a)
	call32: CALLFNcall32, 32)
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x433358)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/usr/lib64/go/1.11/src/reflect/value.go:207 (0x4bd2a3)
	flag.mustBe: panic(&ValueError{methodName(), f.kind()})
/usr/lib64/go/1.11/src/reflect/value.go:865 (0x4c017b)
	Value.FieldByName: v.mustBe(Struct)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:146 (0x99144f)
	computeUpdateDiff: ofv := ov.FieldByName(name).Interface()
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:88 (0x990c61)
	addUpdateRecord: diff := computeUpdateDiff(scope)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:70 (0x990907)
	(*Plugin).addUpdated: addUpdateRecord(scope, p.opts)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/plugin.go:25 (0x9944c3)
	(*Plugin).addUpdated-fm: callback.Update().After("gorm:after_update").Register("loggable:update", p.addUpdated)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:888 (0x970a27)
	(*Scope).callCallbacks: (*f)(scope)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/main.go:467 (0x96079f)
	(*DB).Save: newDB := scope.callCallbacks(s.parent.callbacks.updates).db
/home/vzool/Workspace/seren/webapp/app/http/controllers/country.go:210 (0xb31f01)
	CountryUpdate: if err := db.Save(&data).Error; err != nil {
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/auth.go:55 (0xb562d2)
	Auth.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/hideID.go:32 (0xb56d31)
	HideID.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/cors.go:26 (0xb569b2)
	CORS.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-contrib/gzip@v0.0.1/gzip.go:47 (0xb5239e)
	Gzip.func2: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/secure.go:42 (0xb56fff)
	Secure.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/recovery.go:83 (0x8e3af9)
	RecoveryWithWriter.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/logger.go:240 (0x8e2c60)
	LoggerWithConfig.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:389 (0x8d9fa4)
	(*Engine).handleHTTPRequest: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:351 (0x8d97d1)
	(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/usr/lib64/go/1.11/src/net/http/server.go:2741 (0x7034ea)
	serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/lib64/go/1.11/src/net/http/server.go:1847 (0x6ff735)
	(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:1333 (0x460820)
	goexit: BYTE	$0x90	// NOP

@sas1024
Copy link
Owner

sas1024 commented Sep 25, 2019

For now i'm not able to resolve this issue, because i'm not using this addon anymore.

@vcraescu can you help with this issue?

@vetcher
Copy link
Contributor

vetcher commented Sep 27, 2019

@vzool Hi. I think, the main reason, why it panics, because you use pointers in Update method. Could you post minimal example, that panics on https://play.golang.org ?
In current implementation, diff expects from model to be structure.

Also, as quick fix, I think, you may change code of identity_manager.go:
Before:

func (im *identityManager) save(value, pk interface{}) {
	t := reflect.TypeOf(value)
	newValue := reflect.New(t).Interface()
	err := copier.Copy(&newValue, value)
	if err != nil {
		panic(err)
	}

	im.m[genIdentityKey(t, pk)] = newValue
}

After:

func (im *identityManager) save(value, pk interface{}) {
	t := reflect.Indirect(reflect.ValueOf(value)).Type()
	newValue := reflect.New(t).Interface()
	err := copier.Copy(&newValue, value)
	if err != nil {
		panic(err)
	}

	im.m[genIdentityKey(t, pk)] = newValue
}

@vzool
Copy link
Author

vzool commented Oct 1, 2019

@vetcher I'm not sure now, because I forked it locally and I did changed a lot of things for a degree that I can get rid of identityManager itself.

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

No branches or pull requests

3 participants