Merge pull request #64 from nezuo/no-yield-before-callbacks

Throw error if beforeSave or beforeClose yield

CHANGELOG.md

@@ -3,9 +3,11 @@
 ## Unreleased Changes
 * Files now use the `.luau` extension instead of `.lua`. ([#61])
 * Switched wally realm to `shared`. This means Lapis can be used as a shared or server dependency. ([#62])
+* `beforeClose` and `beforeSave` now throw an error if they yield. For more information, see the PR. ([#64])
 
 [#61]: https://github.com/nezuo/lapis/pull/61
 [#62]: https://github.com/nezuo/lapis/pull/62
+[#64]: https://github.com/nezuo/lapis/pull/64
 
 ## 0.3.2 - August 6, 2024
 * Added `Collection:read` to view a document's data without editing or session locking it. ([#59])

src/Document.luau

@@ -1,5 +1,6 @@
 local freezeDeep = require(script.Parent.freezeDeep)
 local Promise = require(script.Parent.Parent.Promise)
+local noYield = require(script.Parent.noYield)
 
 local function runCallback(document, name, callback)
 	if callback == nil then
@@ -9,7 +10,7 @@ local function runCallback(document, name, callback)
 	document.callingCallback = name
 
 	return Promise.new(function(resolve, reject)
-		local ok, message = pcall(callback)
+		local ok, message = pcall(noYield, callback)
 
 		document.callingCallback = nil
 
@@ -123,7 +124,7 @@ end
 	:::
 
 	:::warning
-	If the beforeSave callback errors, the returned promise will reject and the data will not be saved.
+	If the beforeSave callback yields or errors, the returned promise will reject and the data will not be saved.
 	:::
 
 	@return Promise<()>
@@ -165,7 +166,7 @@ end
 	If called again, it will return the promise from the original call.
 
 	:::warning
-	If the beforeSave or beforeClose callbacks error, the returned promise will reject and the data will not be saved.
+	If the beforeSave or beforeClose callbacks yield or error, the returned promise will reject and the data will not be saved.
 	:::
 
 	@return Promise<()>

src/Document.test.luau

@@ -238,6 +238,18 @@ return function(x)
 	end)
 
 	x.nested("Document:beforeSave", function()
+		x.test("throws when yielding", function(context)
+			local document = context.lapis.createCollection("collection", defaultOptions()):load("document"):expect()
+
+			document:beforeSave(function()
+				task.wait()
+			end)
+
+			shouldThrow(function()
+				document:save():expect()
+			end, "beforeSave callback threw error: thread is not yieldable")
+		end)
+
 		x.test("throws when setting twice", function(context)
 			local document = context.lapis.createCollection("collection", defaultOptions()):load("document"):expect()
 
@@ -299,6 +311,18 @@ return function(x)
 	end)
 
 	x.nested("Document:beforeClose", function()
+		x.test("throws when yielding", function(context)
+			local document = context.lapis.createCollection("collection", defaultOptions()):load("document"):expect()
+
+			document:beforeClose(function()
+				task.wait()
+			end)
+
+			shouldThrow(function()
+				document:close():expect()
+			end, "beforeClose callback threw error: thread is not yieldable")
+		end)
+
 		x.test("throws when setting twice", function(context)
 			local document = context.lapis.createCollection("collection", defaultOptions()):load("document"):expect()
 

src/noYield.luau

@@ -0,0 +1,11 @@
+local function noYield(callback)
+	for _ in
+		function()
+			callback()
+		end
+	do
+		break
+	end
+end
+
+return noYield