Best Practice for `warn()` Messages and Error Context?

Help and Feedback Scripting Support
1

Here’s the relevant part of my code:

local success, err = self:AddRebirth(player, 1, replica)
if not success then
	warn(string.format(
		"[RebirthService | OnServerEvent] AddRebirth failed for %s (UserId: %d). Error: %s",
		player.Name,
		player.UserId,
		tostring(err)
	))
	return
end

function RebirthService:AddRebirth(player: Player, amount: number, replica: Replica?)
	local success, err = ValidationUtils:ValidateNumber(amount, "Amount")
	if not success then
		return false, err
	end

	success, err = ValidationUtils:ValidateMin(amount, 1, "Amount")
	if not success then
		return false, err
	end

	replica, err = DataManagerUtils:ResolveReplicaOrWarn(player, "RebirthService:AddRebirth", replica)
	if not replica then
		return false, err
	end

	local currentRebirth = replica.Data.Stats.Rebirth
	return self:SetRebirth(player, currentRebirth + amount, replica)
end

function Utils:ResolveReplicaOrWarn(player: Player, contextName: string, replica: Replica?): (Replica?, string?)
	local resolvedReplica, err = self:ResolveReplica(player, replica)

	if not resolvedReplica then
		warn(string.format(
			"[%s] Replica not found for %s (UserId: %d). Error: %s", 
			contextName, 
			player.Name, 
			player.UserId,
			tostring(err)
			))
		
		return nil, err
	end

	return resolvedReplica
end

Is this a good way to handle warn() messages?

My goal is to monitor player-related errors through Creator Hub after release, and I want the logs to be easy to trace. Since Creator Hub doesn’t clearly show where an error originated, I’m including a prefix like [RebirthService | OnServerEvent] so I can immediately identify the source.

I also want the warnings to be useful in the in-game Output, so if a player reports a bug in my discord server, they can simply send me their Output log.

The only thing I’m unsure about is including player.Name and player.UserId in the warning. In Creator Hub they don’t seem to display properly and instead show something like <Player>. Would it be better to keep that information anyway, or should I just make the message more generic, like "Failed to add rebirth" or "Failed to process rebirth request" so it doesn’t use player.Name at all?

I’m mainly looking for feedback on whether this logging approach scales well or if there’s a cleaner pattern people commonly use.

1 Like
2

Nobody’s picked this up in a month, so — the reason this feels unresolved is that you have two warn sites for one failure, and they’re fighting each other.

ResolveReplicaOrWarn warns internally, then returns (nil, err). AddRebirth passes that err straight up. OnServerEvent then warns a second time with the same string. One missing replica, two lines in the output, and the second one is the only one carrying the calling context you actually want.

Pick one layer and commit to it. Two options that both work:

Warn at the leaf. The helper owns its own message and returns a plain false/nil with no error string. Call sites stay silent. You get one line, written closest to the thing that failed — but the caller loses the ability to say which operation was in flight.

Warn at the boundary. Rename it to ResolveReplica, drop the internal warn, return (nil, err) and let the top-level handler do the single formatted warn. This is the one I’d pick for your code, because your OnServerEvent block already has the context the leaf doesn’t — player, UserId, and the fact that it was a rebirth that failed.

The name is doing you damage either way: ...OrWarn promises the helper handles messaging, and then you handle it again anyway. If you go with the second option, the rename is most of the fix.

One thing worth checking whichever way you go: your ValidationUtils errors need to be descriptive at the point they’re created. If ValidateMin returns just "Amount" or "validation failed", the boundary warn will faithfully print a string that tells you nothing, and you’ll be back to reading the stack.

3

opsec is just horrible, ur giving out way too much info in the logs, theres no reason to mention what service warned and that it comes in OnServerEvent