kikito/inspect.lua

Incompatible with luvit or the Discordia library

Tenrys opened this issue · 16 comments

I'm not sure how to formulate this issue, because I don't know if I should be using this with luvit in general, but here I go.

I'm using the Discordia library and it seems doing something as simple as inspect(client) will give the following error:

inspect.lua:246: attempt to compare number with nil

(here's the erroring line)

client being a discordia.Client object here.

I would have to investigate this in more detail. In order to fix this issue I would need to be able to reproduce it in the first place. Do you think you can produce a minimal example showing the issue?

I'll try to find a way to reproduce it without having to create a bot.

Okay, I don't think I can reproduce this without using discordia and a bot token.
https://discordapp.com/developers/applications/me

local discordia = require('discordia')
local client = discordia.Client()

client:run('Bot INSERT_TOKEN_HERE')

inspect(client)

That should be enough, if it does not error it might require the bot to be in a guild.

Hi there @Tenrys,

Sorry it took me a little while to get back to you on this. I just pushed a change to master that might solve your issue with inspect.lua. Could you please that new version in discord?

Got the same error, different line though
libs/inspect.lua:247: attempt to compare number with nil

Wow that was fast. Please try again. I had a bit of a problem with git and had to repush.

Well now it "works", it returns a stack overflow.

Ok. Thanks for the quick response. I think something is not quite right with the change I made. I thought it was correct because my tests were locally passing but now it seems that I had a version of inspect installed via luarocks and it was used on the tests instead of using my dev one. I'm checking.

I have removed the change from master and will continue working on a branch (as I should have). Apologies for the inconveniences.

It's all good, I made a simple similar function for my own needs in the meantime and it works alright.

For reference, here is what I'm getting when pretty-printing a newly created discordia client using vinspect: https://gist.github.com/mpeterv/64baa14b072a35077c7426053bcc386b

So the problem is that inspect iterates over table pairs in two different ways and gets conflicting results. When counting table appearances it uses pairs, but when pretty-printing tables it first iterates over array keys using rawget, and only uses pairs for remaining keys.

The problem is that a table within discordia client (client[6]) overrides __pairs so that pairs returns no keys. As a result, tables within actual array part are not counted as appeared and so when they are discovered during pretty printing the error is thrown.

Testing:

local discordia = require "discordia"
local client = discordia.Client()

print("pairs():")

for key, value in pairs(client[6]) do
   print(key, value)
end

print("rawget():")

for i = 1, 4 do
   print(i, rawget(client[6], i))
end

print("raw pairs():")

getmetatable(client[6]).__pairs = nil

for key, value in pairs(client[6]) do
   print(key, value)
end

Output:

pairs():
rawget():
1	0
2	table: 0x418f0cc8
3	class User
4	Client
raw pairs():
1	0
2	table: 0x418f0cc8
3	class User
4	Client

Replacing pairs(t) with next, t everywhere in inspect fixes the issue.

(I wrote vinspect for 1. interactive inspection 2. situations like this where everything is broken (inspired by Penlight overriding type with something returning tables... vinspect somewhat handles that), not sure if inspect needs to be that defensive particularly since it wants to avoid stuff like debug.metatable, but in cases like this it couldn't hurt.)

Wow, thanks for the analysis! I will certainly look at using next instead of pairs.

@Tenrys can you try with the implementation that I put on the rawpairs branch?

https://github.com/kikito/inspect.lua/blob/rawpairs/inspect.lua

I changed it so that it always ignores __pairs and __ipairs, as mentioned by @mpeterv .

I have merged rawpairs in the main branch now - it was solving a problem even if it didn't solve the problem (which I think is solved as well, but I'll leave this open for a while in case you want to try it out)