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

[ SQL ] Misleading error message on email update #386

Closed
danieltamas opened this issue Oct 22, 2019 · 10 comments
Closed

[ SQL ] Misleading error message on email update #386

danieltamas opened this issue Oct 22, 2019 · 10 comments

Comments

@danieltamas
Copy link

When trying to update the email field in the users table via a RPC function, one gets the duplicate key constraint error in the docker logs.

The problem here is the error is not correct nor the docs. In order to update the email field on the users table one needs to add the update_time = now() to the update query, after which the query executes correctly.

It should be added in the docs and the error message from SQL should be more specific.

Description

Example: When updating a user's email address, the RPC call fails 500 error

Steps to Reproduce

Create a simple RPC function and call it from the front end to update the email for the current user.

Expected Result

The email should be updated

Actual Result

The server responds with 500 error

Context

  • Browser / Chrome

Your Environment

  • Nakama: 2.7.0
@mofirouz
Copy link
Member

mofirouz commented Oct 22, 2019

What does your RPC function look like?

@danieltamas
Copy link
Author

This was before we added update_time = now() - which didn't work.

local nk = require("nakama")
local function update_account_custom(_, payload)
local json = nk.json_decode(payload)

local query = [[UPDATE users SET first_name = $1, last_name = $2, username = $3, phone = $4, 
location = $5, email = $6  WHERE id = $7]]
local parameters = {json.firstName, json.lastName, json.username, json.phone, 
nk.json_encode(json.country), json.email, json.userId}
local rows = nk.sql_exec(query, parameters)

return nk.json_encode(rows)
end

nk.register_rpc(update_account_custom, "update_account_custom")

Then we changed the query to
local query = [[UPDATE users SET first_name = $1, last_name = $2, username = $3, phone = $4, location = $5, email = $6, update_time = now() WHERE id = $7]]

@zyro
Copy link
Member

zyro commented Oct 24, 2019

We do not control the error messages from SQL. The nk.sql_exec function gives you direct database access, the error you see is exactly what the database returns.

In this case you should only get an error there if the email address is already in use by a different user. When you first tried the query was it with a different user ID (or a different email address) than the 2nd attempt?

@danieltamas
Copy link
Author

I see. Well the email was the same and the response was a constraint violation that had nothing to do with the updated_time field. This is where the confusion.

Below le' error.

{"level":"error","ts":"2019-10-24T10:42:33.814Z","msg":"Runtime RPC function caused an error","id":"update_account_custom","error":"/nakama/data/modules/update_account_custom.lua:7: sql exec error: ERROR: duplicate key value (email)=('t*****k@gmail.com') violates unique constraint \"users_email_key\" (SQLSTATE 23505)\nstack traceback:\n\t[G]: in function 'sql_exec'\n\t/nakama/data/modules/update_account_custom.lua:7: in main chunk\n\t[G]: ?","stacktrace":"github.com/heroiclabs/nakama/v2/server.(*RuntimeProviderLua).Rpc\n\tgithub.com/heroiclabs/nakama/v2@/server/runtime_lua.go:1013\ngithub.com/heroiclabs/nakama/v2/server.NewRuntimeProviderLua.func3.1\n\tgithub.com/heroiclabs/nakama/v2@/server/runtime_lua.go:179\ngithub.com/heroiclabs/nakama/v2/server.(*ApiServer).RpcFuncHttp\n\tgithub.com/heroiclabs/nakama/v2@/server/api_rpc.go:160\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2007\ngithub.com/gorilla/mux.(*Router).ServeHTTP\n\tgithub.com/gorilla/mux@v1.7.3/mux.go:212\ngo.opencensus.io/plugin/ochttp.(*Handler).ServeHTTP\n\tgo.opencensus.io@v0.22.1/plugin/ochttp/server.go:86\ngithub.com/heroiclabs/nakama/v2/server.decompressHandler.func1\n\tgithub.com/heroiclabs/nakama/v2@/server/api.go:455\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2007\ngithub.com/gorilla/handlers.CompressHandlerLevel.func1\n\tgithub.com/gorilla/handlers@v1.4.2/compress.go:148\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2007\ngithub.com/heroiclabs/nakama/v2/server.StartApiServer.func5\n\tgithub.com/heroiclabs/nakama/v2@/server/api.go:210\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2007\ngithub.com/heroiclabs/nakama/v2/server.StartApiServer.func6\n\tgithub.com/heroiclabs/nakama/v2@/server/api.go:221\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2007\ngithub.com/gorilla/mux.(*Router).ServeHTTP\n\tgithub.com/gorilla/mux@v1.7.3/mux.go:212\ngithub.com/gorilla/handlers.(*cors).ServeHTTP\n\tgithub.com/gorilla/handlers@v1.4.2/cors.go:138\nnet/http.serverHandler.ServeHTTP\n\tnet/http/server.go:2802\nnet/http.(*conn).serve\n\tnet/http/server.go:1890"}

@zyro
Copy link
Member

zyro commented Oct 24, 2019

Was the user ID different between the two attempts? The error looks correct if you try to set an email address on a user ID when it's already being used by a different user ID. I suspect the updated_time is not relevant here.

@danieltamas
Copy link
Author

Nope. same attempt, same everything. The only diff is updated_time. This we noticed in the nakama source files when updating the account and decided to add it to the query. Once we did, everything worked as expected.

@zyro
Copy link
Member

zyro commented Oct 24, 2019

Sounds strange, I can't think of a reason the update time would make a difference since the uniqueness constraint is only defined on the email column alone.

I'm not sure what version of the database you're running or how your schema modifications might play into it but it doesn't seem to be a Nakama server issue so I'll close this for now.

@zyro zyro closed this as completed Oct 24, 2019
@danieltamas
Copy link
Author

Well neither do we and that constraint about the email mislead us.

The DB version is 19.1.5

@zyro
Copy link
Member

zyro commented Oct 24, 2019

I should also mention for the record we don't recommend modifying Nakama's own tables, and if you add any additional tables there should be no constraints or relationships to Nakama's own tables. Each operation in Nakama is designed specifically around a known schema and we can't guarantee what would happen from a data integrity or performance perspective if 'known' tables are changed.

You should also be careful about storing any personal information (like phone number) in the database outside of Nakama's known schema. The built-in GDPR support won't be aware of them if you do, so you'll be responsible for GDPR compliance outside of the available tooling.

@danieltamas
Copy link
Author

danieltamas commented Oct 28, 2019

fair enough and thank you for the pointers. The GDPR compliance is also treated on our end so that is taken care of.

The current issue we described is not related however to the above ( useful ) mentions.

The problem is replicated ( we just did the test ) on an unmodified table as well, plus the thrown error is not related to any of the added columns. It may be a constraint on the DB that acts funny.

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