Make new/async more explicit

This commit is contained in:
Eryn Lynn 2019-09-18 16:42:15 -04:00
parent 0139ed512f
commit ffaff8a4b1
5 changed files with 52 additions and 78 deletions

View file

@ -1,3 +1,10 @@
# 2.3.0
- Remove `Promise.spawn` from the public API.
- `Promise.async` still inherits the behavior from `Promise.spawn`.
- `Promise.async` now wraps the callback in `pcall` and rejects if an error occurred.
- `Promise.new` has now has an explicit error message when attempting to yield inside of it.
# 2.2.0
- `Promise.promisify` now uses `coroutine.wrap` instead of `Promise.spawn`

View file

@ -76,12 +76,12 @@ docs:
- name: async
tags: [ 'constructor' ]
desc: |
The same as [[Promise.new]], except it implicitly uses [[Promise.spawn]] internally. Use this if you want to yield inside your Promise body.
The same as [[Promise.new]], except it allows yielding. Use this if you want to yield inside your Promise body.
If your Promise body does not need to yield, such as when attaching `resolve` to an event listener, you should use [[Promise.new]] instead.
::: tip
Promises created with [[Promise.async]] don't begin executing until the next `RunService.Heartbeat` event, even if the executor function doesn't yield itself. <a href="/roblox-lua-promise/lib/Details.html#yielding-in-promise-executor">Learn more</a>
Promises created with [[Promise.async]] don't begin executing until the next `RunService.Heartbeat` event, even if the executor function doesn't yield itself. This is to ensure that Promises produced from a function are either always synchronous or always asynchronous. <a href="/roblox-lua-promise/lib/Details.html#yielding-in-promise-executor">Learn more</a>
:::
```lua
@ -192,16 +192,6 @@ docs:
returns:
- type: boolean
desc: "`true` if the given `object` is a Promise."
- name: spawn
desc: Spawns a thread with predictable timing. The callback will be called on the next `RunService.Heartbeat` event.
static: true
params:
- name: callback
type:
kind: function
params: "...: ...any?"
- name: "..."
type: "...any?"
# Instance methods
- name: andThen

View file

@ -68,7 +68,7 @@ end)
You must observe the result of a Promise, either with `catch` or `finally`, otherwise an unhandled Promise rejection warning will be printed to the console.
If an error occurs while executing the Promise body, the Promise will be rejected automatically with the error text if it's in a synchronous Promise. Otherwise, the error won't be caught.
If an error occurs while executing the Promise body, the Promise will be rejected automatically with the error text and a trace back.
## Chaining
@ -122,31 +122,20 @@ Promise.async(function(resolve)
end)
```
`Promise.async` uses `Promise.new` internally, except it wraps the Promise executor with <ApiLink to="Promise.spawn" />.
`Promise.async` uses `Promise.new` internally, except it allows yielding while `Promise.new` does not.
`Promise.async` is sugar for:
`Promise.async` attaches a one-time listener to the next `RunService.Heartbeat` event to fire off the rest of your Promise executor, ensuring it always waits at least one step.
```lua
Promise.new(function(resolve, reject, onCancel)
Promise.spawn(function()
-- ...
end)
end)
```
### Promise.spawn
`Promise.spawn` attaches a one-time listener to the next `RunService.Heartbeat` event to fire off the rest of your Promise executor, ensuring it always waits at least one step.
The reason `Promise.spawn` includes this wait time is to ensure that your Promises have consistent timing. Otherwise, your Promise would run synchronously up to the first yield, and asynchronously afterwards. This can often lead to undesirable results. Additionally, Promises that never yield can resolve completely synchronously, and this can lead to predictable, but often unexpected timing issues. Thus, we use `Promise.spawn` so there is always a guaranteed yield before execution.
The reason `Promise.async` includes this wait time is to ensure that your Promises have consistent timing. Otherwise, your Promise would run synchronously up to the first yield, and asynchronously afterwards. This can often lead to undesirable results. Additionally, Promise executors that only sometimes yield can lead to unexpected timing issues. Thus, we use `Promise.async` so there is always a guaranteed yield before execution.
::: danger Don't use regular spawn
`spawn` might seem like a tempting alternative to `Promise.spawn` here, but you should **never** use it!
Using `spawn` inside `Promise.new` might seem like a tempting alternative to `Promise.async` here, but you should **never** use it!
`spawn` (and `wait`, for that matter) do not resume threads at a consistent interval. If Roblox has resumed too many threads in a single Lua step, it will begin throttling and your thread that was meant to be resumed on the next frame could actually be resumed several seconds later. The unexpected delay caused by this behavior will cause cascading timing issues in your game and could lead to some potentially ugly bugs.
:::
### When to use `Promise.new`
In some cases, it is desirable for a Promise to execute completely synchronously. If you don't need to yield in your Promise executor, and you are aware of the timing implications of a completely synchronous Promise, then it is acceptable to use `Promise.new`.
In some cases, it is desirable for a Promise to execute completely synchronously. If you don't need to yield in your Promise executor, then you should use `Promise.new`.
For example, an example of a situation where it might be appropriate to use Promise.new is when resolving after an event is fired.
@ -168,7 +157,7 @@ If you attach a `:andThen` or `:catch` handler to a Promise after it's been canc
::: warning
If you cancel a Promise immediately after creating it without yielding in between, the fate of the Promise is dependent on if the Promise handler yields or not. If the Promise handler resolves without yielding, then the Promise will already be settled by the time you are able to cancel it, thus any consumers of the Promise will have already been called and cancellation is not possible.
If the Promise does yield, then cancelling it immediately *will* prevent its resolution. This is always the case when using `Promise.async`/`Promise.spawn`.
If the Promise does yield, then cancelling it immediately *will* prevent its resolution. This is always the case when using `Promise.async`.
:::
Attempting to cancel an already-settled Promise is ignored.

View file

@ -3,7 +3,6 @@
]]
local RunService = game:GetService("RunService")
local PROMISE_DEBUG = false
--[[
Packs a number of arguments into a table and returns its length.
@ -16,30 +15,24 @@ local function pack(...)
return len, { ... }
end
--[[
wpcallPacked is a version of xpcall that:
* Returns the length of the result first
* Returns the result packed into a table
* Passes extra arguments through to the passed function; xpcall doesn't
* Issues a warning if PROMISE_DEBUG is enabled
]]
local function wpcallPacked(f, ...)
local argsLength, args = pack(...)
local function packResult(...)
local result = (...)
local body = function()
return f(unpack(args, 1, argsLength))
return result, pack(select(2, ...))
end
local function ppcall(callback, ...)
local co = coroutine.create(callback)
local ok, len, result = packResult(coroutine.resume(co, ...))
if ok and coroutine.status(co) ~= "dead" then
error("Yielding inside Promise.new is not allowed! Use Promise.async or create a new thread in the Promise executor!", 2)
elseif not ok then
result[1] = debug.traceback(result[1], 2)
end
local resultLength, result = pack(xpcall(body, debug.traceback))
-- If promise debugging is on, warn whenever a pcall fails.
-- This is useful for debugging issues within the Promise implementation
-- itself.
if PROMISE_DEBUG and not result[1] then
warn(result[2])
end
return resultLength, result
return ok, len, result
end
--[[
@ -48,13 +41,12 @@ end
]]
local function createAdvancer(callback, resolve, reject)
return function(...)
local resultLength, result = wpcallPacked(callback, ...)
local ok = result[1]
local ok, resultLength, result = ppcall(callback, ...)
if ok then
resolve(unpack(result, 2, resultLength))
resolve(unpack(result, 1, resultLength))
else
reject(unpack(result, 2, resultLength))
reject(unpack(result, 1, resultLength))
end
end
end
@ -183,9 +175,8 @@ function Promise.new(callback, parent)
return self._status == Promise.Status.Cancelled
end
local _, result = wpcallPacked(callback, resolve, reject, onCancel)
local ok = result[1]
local err = result[2]
local ok, _, result = ppcall(callback, resolve, reject, onCancel)
local err = result[1]
if not ok and self._status == Promise.Status.Started then
reject(err)
@ -195,25 +186,20 @@ function Promise.new(callback, parent)
end
--[[
Promise.new, except Promise.spawn is implicit.
Promise.new, except pcall on a new thread is automatic.
]]
function Promise.async(callback)
return Promise.new(function(...)
return Promise.spawn(callback, ...)
end)
end
local traceback = debug.traceback()
return Promise.new(function(resolve, reject, onCancel)
local connection
connection = RunService.Heartbeat:Connect(function()
connection:Disconnect()
local ok, err = pcall(callback, resolve, reject, onCancel)
--[[
Spawns a thread with predictable timing.
]]
function Promise.spawn(callback, ...)
local args = { ... }
local length = select("#", ...)
local connection
connection = RunService.Heartbeat:Connect(function()
connection:Disconnect()
callback(unpack(args, 1, length))
if not ok then
reject(err .. "\n" .. traceback)
end
end)
end)
end
@ -661,8 +647,10 @@ function Promise.prototype:_finalize()
end
-- Allow family to be buried
self._parent = nil
self._consumers = nil
if not Promise.TEST then
self._parent = nil
self._consumers = nil
end
end
return Promise

View file

@ -1,5 +1,6 @@
return function()
local Promise = require(script.Parent)
Promise.TEST = true
local function pack(...)
local len = select("#", ...)
@ -72,7 +73,6 @@ return function()
expect(promise:getStatus()).to.equal(Promise.Status.Rejected)
expect(promise._values[1]:find("hahah")).to.be.ok()
-- Loosely check for the pieces of the stack trace we expect
expect(promise._values[1]:find("init.spec")).to.be.ok()
expect(promise._values[1]:find("new")).to.be.ok()