evaera / roblox-lua-promise

Promise implementation for Roblox
https://eryn.io/roblox-lua-promise/
MIT License
281 stars 78 forks source link

Use pcall() on Promise.is when looking for the .andThen function #22

Closed ddavness closed 4 years ago

ddavness commented 4 years ago

Some tables might have strict metatables that error if a non-existant member is indexed, causing the chain to fail.

Minimal reproduction sample:

local Promise = require(--[[ Promise path here ]])
local noob = setmetatable({Health = 100}, {
    __index = function()
        error("You cant index non-existant stuff!")
    end
}

-- Promise will instantly reject
Promise.new(function(resolve, reject)
    resolve(noob)
end):andThen(function(monster)
    print(monster.Health)
end)
evaera commented 4 years ago

This seems like a real edge case that's going to add some performance overhead. Can you give me a real world example that caused this issue to occur for you?

ddavness commented 4 years ago

I'm currently writing an http library and the objects/classes/whatever represented try to emulate the behavior of userdatas like Vector3 (where indexing non-existant members will immediately throw) via a metatable that throws.

However, unlike the actual userdatas where type(object) == "userdata", it's still a table and goes through the first check (but fails the second because it throws).

I admit that there might be better solutions to this issue rather than employing pcall() right away (but pcall() is hands down the easiest to implement).

I tried employing rawget but that will break the whole module. On the other hand, is the performance overhead of a pcall() really that big/noticeable of an issue?