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

Latest update breaks "reply", causes crash. #76

Closed
dennisjenkins75 opened this issue Apr 22, 2023 · 7 comments
Closed

Latest update breaks "reply", causes crash. #76

dennisjenkins75 opened this issue Apr 22, 2023 · 7 comments
Labels
Bug Something isn't working
Milestone

Comments

@dennisjenkins75
Copy link
Contributor

  • Server runs "multicraft 2.0.0", not canonical "minetest 5.x"

I am unable to reliably reproduce this though. While trying to reproduce it, I observed that when a sender clicks "send" to send the mail, it just stays in their "Drafts" folder and is instantly delivered to the recipient (if both are logged in and in "/mail" at the same time) until they log out and then back in. Anyway, the stack trace might be useful.

The problem appeared ~7h ago after I updated to the latest mail mod. Sadly, the git revision history shows lots of changes, and without a repro, I am unable to bisect to find which change introduced this bug.

ServerError: AsyncErr: ServerThread::run Lua: Runtime error from mod 'mail' in callback on_playerReceiveFields(): /home/1hit/worlds/world/worldmods/mail/ui/message.lua:48: attempt to concatenate field 'body' (a nil value)
stack traceback:
        /home/1hit/worlds/world/worldmods/mail/ui/message.lua:48: in function 'reply'
        /home/1hit/worlds/world/worldmods/mail/ui/message.lua:105: in function </home/1hit/worlds/world/worldmods/mail/ui/message.lua:87>
        /home/1hit/multicraft/bin/../builtin/game/register.lua:441: in function </home/1hit/multicraft/bin/../builtin/game/register.lua:425>
@S-S-X
Copy link
Member

S-S-X commented Apr 22, 2023

It seems like register_on_player_receive_fields isn't checking incoming fields and that should be done always.
Doing so will fix this issue and also prevent crashing server with custom client.

However problem itself wont go away by doing that and it would be good to find reason for this behavior.

But then also I guess bug reports will be made if some of the buttons wont work correctly sometimes so user input validations / crash could be fixed first and take a look at root cause afterwards. (also link #70)

@S-S-X S-S-X added the Bug Something isn't working label Apr 22, 2023
@BuckarooBanzay
Copy link
Member

looks like there is a message in the store with a nil body field, do you have the tools to check the mod-storage for any such mails?

Another option would be to sprinkle in a few asserts with a meaningful message, player-name and mail-id and catch the error when it pops up next.

@dennisjenkins75
Copy link
Contributor Author

looks like there is a message in the store with a nil body field, do you have the tools to check the mod-storage for any such mails?

Prior to moving the messages into mod-storage, I could use "/usr/bin/jq" to find weird messages, but now I would need to craft something custom.

Another option would be to sprinkle in a few asserts with a meaningful message, player-name and mail-id and catch the error when it pops up next.

@S-S-X
Copy link
Member

S-S-X commented Apr 22, 2023

Prior to moving the messages into mod-storage, I could use "/usr/bin/jq" to find weird messages, but now I would need to craft something custom.

You should still be able to do it just like before but just instead of reading file you have to read database, something like:

psql -d mtdb -AXwtc "SELECT value FROM mod_storage WHERE key = 'key_you_want' LIMIT 1" | jq 'jq query you want'

Sure not as straightforward as it was before, that's trade off for having better and safer storage system.

@Athozus Athozus added this to the 1.1.4 milestone Apr 26, 2023
@Athozus
Copy link
Member

Athozus commented Apr 27, 2023

@dennisjenkins75 Some things about replying were fixed, could you pull from master and tell us if it's still happening ? The fixes includes reply button in message view, notably.

@dennisjenkins75
Copy link
Contributor Author

I updated EdenLost to 7449aac just now. I'll report if it crashes again.

@Athozus
Copy link
Member

Athozus commented Apr 28, 2023

OK. If you have no problems for 2 days I consider it is fixed, okay ?

@Athozus Athozus closed this as completed Apr 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants