fix: Fix PPE Actors termination on budget exhaustion

[janbuchar]

• closes PPE Actors never terminate if the user doesn't check the return value of Actor.charge #572

When charging would exceed the budget, deliberately overcharge by one event so the platform detects the overspend and terminates the run. Previously, the SDK would silently cap to zero, meaning the platform never saw an over-budget charge and the run would hang indefinitely.

Applied to both charge() and pushData() paths. For pushData(), the overcharge only triggers when the budget is fully depleted (zero remaining) - if there's partial budget left, items are still capped normally to what the budget allows.

[barjin]

Thank you @janbuchar , the changes seem pretty sound to me 👍

Some notes here:

[barjin]

For pushData(), the overcharge only triggers when the budget is fully depleted (zero remaining) - if there's partial budget left, items are still capped normally to what the budget allows

What is the reasoning behind this (different behaviour with events and dataset items)? If I understand correctly, now

/// budget is $5 USD
await Actor.charge({name: 'oneDollar', count: 10}) // this will charge $6 USD, run aborts

/// budget is $5 USD
await Actor.pushData(new Array(10).fill({...})) // this will charge $5 USD, run continues running
/// ... fetch 10 more items from a pricy third-party API
await Actor.pushData(new Array(10).fill({...})) // this will charge $1 USD, run aborts

[janbuchar]

Well, the general idea is that 1. you should always check the return value of the charge / pushData calls and 2. pushData already trims off stuff that the user cannot afford.

So one partial pushData is legit usage, but if you try after already spending the whole budget, you're almost certainly not checking your return values and deserve to be nudged over the limit.

[barjin]

I suppose you swayed me with the pushData... but why shouldn't Actor.charge work the same then? Let's say somebody does this:

/// budget is $5 USD
const r = await Actor.charge({ name: 'oneDollar', count: 10 });

if (r.chargedCount === 0) process.exit();
for (let i = 0; i < r.chargedCount; i++) { provideAnExpensiveServiceToTheUser(); }

If I were only to follow the documentation we now have (e.g., the .charge() JSDoc), this feels clean, but it will actually rip off the user (killing the Actor before it can provide any service).

Do we have a docs article on best PPE practices somewhere? (e.g., whether to call Actor.charge before or after providing the service, what should the termination condition be etc.)?

[janbuchar]

I guess that https://docs.apify.com/sdk/js/docs/next/concepts/pay-per-event is the best thing we have now.

Using the count parameter like this is not ideal, as you correctly point out. If the work done by the Actor can be meaningfully split into N chunks, you should issue N Actor.charge calls. I guess we should make this clear in the jsdoc...

[barjin]

If the Platform doesn't abort immediately, this will keep charging for at least one item on every .charge() call.

This doesn't matter, because the over-limit charges are not billed towards the user's account anyway, right?

[janbuchar]

Exactly, yes.

[janbuchar]

I guess we could add one more condition - don't do this if the run is already strictly over the budget?

[mhamas]

Suggestion: I think this wouldn't work correctly if you wanted to push e.g. 5 items, each worth $0.5, with the budget $2.1. In such a case, this logic would compute maxChargedCount to be 4, only min(4, 5) = 4 items would be kept, and the overcharge by 1 wouldn't trigger.