Logic Error in the ProcessReceipt Example Code will cause player to get the same paid items with every server join after purchase

[knightmb1 (KnightMB)]

This sample code below has a logic error starting on this line:
local success, isPurchaseRecorded = pcall(function()
The isPurchaseRecorded will always return nil instead of true upon successful granting of the item to the player if the playerProductKey sent is not in the purchaseHistoryStore. Usually, this just means the player would get a double-grant once because then this would trigger the Enum.ProductPurchaseDecision.PurchaseGranted, record the purchase and not be an issue anymore. But because isPurchaseRecorded always returns nil no matter if the product is granted or not, it creates a loop where the player gets granted the products every server visit instead of one time.

This is also causing errors “Transform function error Callbacks cannot yield”

The original documentation sample code did not have this issue of infinite granting products to the player every server visit.

Current Sample Documentations Code:

local MarketplaceService = game:GetService("MarketplaceService")
local DataStoreService = game:GetService("DataStoreService")
local Players = game:GetService("Players")

-- Data store setup for tracking purchases that were successfully processed
local purchaseHistoryStore = DataStoreService:GetDataStore("PurchaseHistory")

-- Table setup containing product IDs and functions for handling purchases
local productFunctions = {}

-- ProductId 123123 for a full heal
productFunctions[123123] = function(_receipt, player)

	-- Logic for the player buying a full heal
	if player.Character and player.Character:FindFirstChild("Humanoid") then

		-- Heal the player to full health
		player.Character.Humanoid.Health = player.Character.Humanoid.MaxHealth

		-- Indicate a successful purchase
		return true
	end
end

-- ProductId 456456 for 100 gold
productFunctions[456456] = function(_receipt, player)

	-- Logic for player buying 100 gold
	local stats = player:FindFirstChild("leaderstats")
	local gold = stats and stats:FindFirstChild("Gold")
	if gold then
		gold.Value = gold.Value + 100

		-- Indicate a successful purchase
		return true
	end
end

-- The core 'ProcessReceipt' callback function
local function processReceipt(receiptInfo)

	-- Check the data store to determine if the product was already granted
	local playerProductKey = receiptInfo.PurchaseId
	local purchased = false
	local success, result, errorMessage

	success, errorMessage = pcall(function()
		purchased = purchaseHistoryStore:GetAsync(playerProductKey)
	end)

	-- If the purchase is recorded, the product was already granted
	if success and purchased then
		return Enum.ProductPurchaseDecision.PurchaseGranted
	elseif not success then
		error("Data store error:" .. errorMessage)
	end

	-- Update the purchase record
	local success, isPurchaseRecorded = pcall(function()
		return purchaseHistoryStore:UpdateAsync(playerProductKey, function(alreadyPurchased)
			if alreadyPurchased then
				return true
			end

			-- Find the player who made the purchase in the server
			local player = Players:GetPlayerByUserId(receiptInfo.PlayerId)
			if not player then

				-- The player probably left the game
				-- If the player returns, the callback is called again
				return nil
			end

			local handler = productFunctions[receiptInfo.ProductId]

			local success, result = pcall(handler, receiptInfo, player)
      
			-- Do not record the purchase if granting the product failed
			if not success or not result then
				error("Failed to process a product purchase for ProductId: " .. tostring(receiptInfo.ProductId) .. " Player: " .. tostring(player) .. " Error: " .. tostring(result))
				return nil
			end

			-- Record the transaction in purchaseHistoryStore
			return true
		end)
	end)

	if not success then
		error("Failed to process receipt due to data store error.")
		return Enum.ProductPurchaseDecision.NotProcessedYet
	elseif isPurchaseRecorded == nil then

		-- Did not update the value in the data store
		return Enum.ProductPurchaseDecision.NotProcessedYet
	else

		-- IMPORTANT: Tell Roblox that the game successfully handled the purchase
		return Enum.ProductPurchaseDecision.PurchaseGranted
	end
end

-- Set the callback; this can only be done once by one script on the server!
MarketplaceService.ProcessReceipt = processReceipt

If you put in some print statements to follow along in the script, you can see towards the end after the product was granted, it still returned nil and thus triggers:
return Enum.ProductPurchaseDecision.NotProcessedYet instead that starts over again if the player leaves and joins the server again.

Expected behavior

Not grant bought products (such as Developer items) to the player every server visit, but only the one time at purchase.

Page URL: https://create.roblox.com/docs/reference/engine/classes/MarketplaceService#ProcessReceipt

[Nightriff]

Thanks for the detailed report! We’ll look into this and let you know the conclusion shortly.

[Nightriff]

@knightmb1

The isPurchaseRecorded will always return nil instead of true upon successful granting of the item to the player if the playerProductKey sent is not in the purchaseHistoryStore.

Can you explain this further? From what I can tell, it should return true when the purchase is granted, in either case of it already being recorded or not being recorded yet.

This will not work if the grant purchase handler yields, which would cause the error Transform function error Callbacks cannot yield because it’s called inside the UpdateAsync function. In such a case, then it should go down the path of returning nil and correctly avoid recording the purchase as granted.

if not success or not result then
	error("Failed to process a product purchase for ProductId: " .. tostring(receiptInfo.ProductId) .. " Player: " .. tostring(player) .. " Error: " .. tostring(result))
	return nil
end

However, if the purchase handler operates correctly (returns true, never yields) then I would expect the following logic flow to occur, ultimately resulting in PurchaseGranted being returned:

-- isPurchaseRecorded will be the first return value from the pcall'd function
local success, isPurchaseRecorded = pcall(function()
		-- The pcall'd function is an anonymous function that returns the results from UpdateAsync.
		-- The first return value from UpdateAsync will be the new value to set for the key
		return purchaseHistoryStore:UpdateAsync(playerProductKey, function(alreadyPurchased)
			-- In the case if it being already recorded as purchased, the new value to set is the same as what it was, which is true.
			if alreadyPurchased then
				return true
				-- more
			
			-- If the player is in the server, the grant purchase handler did not error, and the grant purchase handler returned `true`, then we also return `true` as the new value to set in purchaseHistoryStore
			return true
		-- more

		-- If updating the purchaseHistoryStore value succeeded and the new value was `true`, we know the purchase was granted and can therefore return PurchaseGranted
		return Enum.ProductPurchaseDecision.PurchaseGranted

Am I missing something that you’re seeing?

[Nightriff]

For completeness, I wrote an alternate slightly simpler and more verbosely commented sample.

local MarketplaceService = game:GetService("MarketplaceService")
local DataStoreService = game:GetService("DataStoreService")
local Players = game:GetService("Players")

-- Data store setup for tracking purchases that were successfully processed
local purchaseHistoryStore = DataStoreService:GetDataStore("PurchaseHistory")

local productIdByName = {
	fullHeal = 123123,
	gold100 = 456456,
}

-- A dictionary to look up the handler function to grant a purchase corresponding to a product ID.
-- These functions return true if the purchase was granted successfully.
-- These functions must never yield, since they're called later within an UpdateAsync callback.
local grantPurchaseHandlerByProductId = {}

-- ProductId 123123 for a full heal
grantPurchaseHandlerByProductId[productIdByName.fullHeal] = function(_receipt, player)
	local character = player.Character
	local humanoid = character and character:FindFirstChild("Humanoid")

	-- Ensure the player has a humanoid to heal
	if not humanoid then
		return false
	end

	-- Heal the player to full Health
	humanoid.Health = humanoid.MaxHealth

	-- Indicate a successful grant
	return true
end

-- ProductId 456456 for 100 gold
grantPurchaseHandlerByProductId[productIdByName.gold100] = function(_receipt, player)
	local leaderstats = player:FindFirstChild("leaderstats")
	local goldStat = leaderstats and leaderstats:FindFirstChild("Gold")

	if not goldStat then
		return false
	end

	-- Add 100 gold to the player's gold stat
	goldStat.Value += 100

	-- Indicate a successful grant
	return true
end

-- The core 'ProcessReceipt' callback function
local function processReceipt(receiptInfo)
	local success, result = pcall(
		purchaseHistoryStore.UpdateAsync,
		purchaseHistoryStore,
		receiptInfo.PurchaseId,
		function(isPurchased)
			if isPurchased then
				-- This purchase was already recorded in our data store, so it must have previously been granted.
				-- We avoid calling the grant purchase handler here to prevent granting the purchase twice.

				-- While the value in datastore is already true, we return true again so that the result variable is also true,
				-- which will later be used to return PurchaseGranted from the processReceipt.
				return true
			end

			local player = Players:GetPlayerByUserId(receiptInfo.PlayerId)
			if not player then
				-- Avoids granting the purchase if the player is not in the server.
				-- When they rejoin, this receipt processor will be called again.
				return nil
			end

			local grantPurchaseHandler = grantPurchaseHandlerByProductId[receiptInfo.ProductId]
			if not grantPurchaseHandler then
				-- If there's no handler defined for this product ID, we can't process the purchase.
				-- This will never happen, as long as you set a handler for every product ID sold in your experience.
				warn(`No purchase handler defined for product ID '{receiptInfo.ProductId}'`)
				return nil
			end

			local handlerSucceeded, handlerResult = pcall(grantPurchaseHandler, receiptInfo, player)
			if not handlerSucceeded then
				local errorMessage = handlerResult
				warn(
					`Grant purchase handler errored while processing purchase from '{player.Name}' of product ID '{receiptInfo.ProductId}': {errorMessage}`
				)
				return nil
			end

			local didHandlerGrantPurchase = handlerResult == true
			if not didHandlerGrantPurchase then
				-- The handler did not grant the purchase, so we avoid recording it as granted.
				return nil
			end

			-- The purchase is now granted to the player. Record it as purchased in the data store, which will later be used to return PurchaseGranted from the ReceiptProcessor.
			return true
		end
	)

	if not success then
		local errorMessage = result
		warn(`Failed to process receipt due to data store error: {errorMessage}`)

		-- It's possible at this point that the purchase was granted, but recording it as granted failed due to a data store error.
		-- This is one unavoidable scenario that leads to duplicate granting of the purchase, because processReceipt will be called for this purchase again.

		-- You can mitigate this by keeping another in-memory record of purchases so that the same server will not grant the same purchase twice,
		-- but you'll need a session locking implementation around your datastore to avoid potentially duplicate granting across servers.
		return Enum.ProductPurchaseDecision.NotProcessedYet
	end

	local didGrantPurchase = result == true
	return if didGrantPurchase
		then Enum.ProductPurchaseDecision.PurchaseGranted
		else Enum.ProductPurchaseDecision.NotProcessedYet
end

-- Set the callback; this can only be done once by one script on the server!
MarketplaceService.ProcessReceipt = processReceipt

This mostly follows the same logic (except it skips the initial GetAsync in favor of checking during UpdateAsync) so it probably wouldn’t solve whatever issue you’re encountering if there is one, but let me know if you have further thoughts on this.

[knightmb1 (KnightMB)]

@Nightriff

@knightmb1

The isPurchaseRecorded will always return nil instead of true upon successful granting of the item to the player if the playerProductKey sent is not in the purchaseHistoryStore.

Can you explain this further? From what I can tell, it should return true when the purchase is granted, in either case of it already being recorded or not being recorded yet.

This will not work if the grant purchase handler yields, which would cause the error Transform function error Callbacks cannot yield because it’s called inside the UpdateAsync function. In such a case, then it should go down the path of returning nil and correctly avoid recording the purchase as granted.

Within:
return purchaseHistoryStore:UpdateAsync(playerProductKey, function(alreadyPurchased)
It checks if the purchase was already in the database and returns true if it was, to help prevent the purchase from double activating whatever grant should be given to the player. Since this should not happen for a first time purchase, it safely moves past this. Then it checks if the player is still in the server, safely gets past this.
On this line:
local success, result = pcall(handler, receiptInfo, player)
It does a protected call to grant the item to the player. If the protected calls fails or returns false, then this should catch that:

-- Do not record the purchase if granting the product failed
if not success or not result then
	error("Failed to process a product purchase for ProductId: " .. tostring(receiptInfo.ProductId) .. " Player: " .. tostring(player) .. " Error: " .. tostring(result))
	return nil
end

Finally, past all of those safety checks, you arrive at:

-- Record the transaction in purchaseHistoryStore
return true

I put a print statement right before this return true that printed “product granted successfully” to see if my test was actually making it all the way to the end. So I knew that “isPurchaseRecorded” should have returned true. Instead, it was returning nil.
I put in a print statement for this at:

elseif isPurchaseRecorded == nil then

	-- Did not update the value in the data store
	return Enum.ProductPurchaseDecision.NotProcessedYet
else

This is where the print statement “isPurchasedRecorded: nil” comes from in my screenshot above.

Up back again where this line starts:
local success, isPurchaseRecorded = pcall(function()
The isPurchaseRecorded is always returning nil no matter what happens in the protected call?

I think the Error in the screenshot “Transform function error Callbacks cannot yield” is failing the protected call and why it always returns nil.

My guess is that this line is where it fails. So it never actually writes to the DB and thus always triggers a “Enum.ProductPurchaseDecision.NotProcessedYet” that will “reset” when the player leaves and enters the server again, causing Roblox to “retry” the grant.
purchaseHistoryStore:UpdateAsync(playerProductKey, function(alreadyPurchased)

This is why the player always gets the item grant over and over every server visit, Roblox thinks the transaction grant failed because Roblox is being told this by the failing DB update but at the same time, it actually made it all the way past the functions that “grant” the item to the player, so that is why they get the same stuff every server visit.

[knightmb1 (KnightMB)]

@Nightriff

This mostly follows the same logic (except it skips the initial GetAsync in favor of checking during UpdateAsync) so it probably wouldn’t solve whatever issue you’re encountering if there is one, but let me know if you have further thoughts on this.

I was finally able to track down the error.
This line:

return purchaseHistoryStore:UpdateAsync(playerProductKey, function(alreadyPurchased)

Is producing the error, “Transform function error Callbacks cannot yield” because during the “grant” part of the function, I am calling GetAsync() to check on a related balance for the player. From the documentation, this is a yielding function.

I’ve found the source of my error.

I guess my next question would be, should this code sample be rewritten not to fail on yields or should some documentation be put into it instead to warn developers about a potential error condition should they use a yielding function within the grant function?

Also, the extra verbose documentation is never a bad thing from my experience.

[Nightriff]

Indeed, code may not yield inside an UpdateAsync function. Since the purchase grant handler is called within UpdateAsync, that function must not yield either.

The ideal solution to this is to use a datastore implementation that includes caching and session locking, so that you keep a copy of the player’s data in memory, and guarantee that it’s only modified in that server via session locking, so you know your cache is up to date.

That way, when you need to fetch the latest player data, it can be fetched from your cache in server memory, which is immediate instead of asynchronous.

As far as rewriting the sample such that the grant purchase handler would be called outside of the UpdateAsync function, let me talk to some folks and think about this one a little more. My initial thought here is that the most problematic edge case happens when the reward grant handler is called, succeeds, and then the receipt fails to save to datastore. This would then allow the receiptprocessor to be called again and grant the reward a second time.

BUT… as I understand it, this can already happen, because even if the first “get” part of the UpdateAsync call succeeds (i.e. it enters the update function callback), I think the “Set” part can still fail, which results in the same problem (as I documented in the comments of my rewritten sample).

I’ll follow up once I confirm the best practice here.

[knightmb1 (KnightMB)]

@Nightriff

As far as rewriting the sample such that the grant purchase handler would be called outside of the UpdateAsync function, let me talk to some folks and think about this one a little more. My initial thought here is that the most problematic edge case happens when the reward grant handler is called, succeeds, and then the receipt fails to save to datastore. This would then allow the receiptprocessor to be called again and grant the reward a second time.

As far as sample code goes, I believe that a lot of developers (myself included years ago), the first time they need to lookup how to handle an in-game purchase, we are going to land on that sample code. Like many developers (myself included ), when we see the sample code, we pay more attention to what happens at the top, where all the “granting” takes places. Since it comes from Roblox, we do see all the safety checks below and figure “Well, Roblox has all the possible errors covered, time to move on…” and would probably not think along the lines of “well, technically, if I run a yielding function in this exact spot, I break it.”

I feel that for the future of other Developers, changing it to not be “yielding function sensitive” would save a lot of time and debugging. I did this in my spare time and it still took a few days of tinkering to really pinpoint what I was doing wrong. Since it works and even works (as far as granting) when you break it, that is where all the confusion begins.

Because I was helping someone else, I actually re-wrote the whole sample myself to not have any issues with yielding functions. We both needed to have the ability to do some DB calls in the grant functions (further down the line in our own code that is) and basically I know what the issue is and how to avoid it. I would feel bad though if this just comes up every so often because someone else runs into the same issue and we all just tell them “either re-write the sample or don’t use yielding functions in this part of the sample”

[Nightriff]

For some additional reading to deepen your understanding on the subjects of both proper receipt processing and data storage (they go hand-in-hand), I highly recommend this article that explains the common pitfalls and best practices in detail:

Implement player data and purchasing systems | Documentation - Roblox Creator...

It’s challenging to include a fully functional implementation in a short code sample for this, because to implement with best practice, it needs to be a part of a larger data storage system. It’s unfortunately quite tricky to get right. I think this sample could be improved though, so I’ll continue looking into updating it.

[knightmb1 (KnightMB)]

@Nightriff

It’s challenging to include a fully functional implementation in a short code sample for this, because to implement with best practice, it needs to be a part of a larger data storage system. It’s unfortunately quite tricky to get right. I think this sample could be improved though, so I’ll continue looking into updating it.

You bring up a good point. The sample might be trying to do too much at once. It shows a great example of how the purchase process flows. All of the extra safety checks might be giving a false sense of security that the developer need only worry about the granting functions up top. Once I found out what I was doing wrong, it seemed so obvious then.

I guess it is really down to what the sample code can teach. I’ve done my share of code documentation jobs over the years. I’m glad you and the rest of the team are willing to discuss it openly. I wasn’t so lucky in my past career when it came to documation dispute discussions being civil.