fix: Keep track of synthetic apify-default-dataset-item events #528

janbuchar · 2025-12-12T16:42:29Z

closes Track the number of apify-default-dataset-item events locally #523

janbuchar · 2025-12-12T16:44:18Z

@mhamas @metalwarrior665 this proved to be way more complicated than previously thought. I'll try to simplify it on Monday.

janbuchar · 2025-12-12T20:46:42Z

An alternative solution to this could be just screaming bloody murder in case that there is a nonzero price for apify-default-dataset-item and another event type is present in the configuration. This would force the developer to start using a custom event to charge for dataset items. It is possible to implement it on the SDK level as well as on platform.

janbuchar · 2025-12-15T09:01:12Z

Another note, even if we manage to fix this on the SDK level, it'll be difficult to make all users update. Granted, if they use PPE, they should be on a recent version already so an update should be easy enough, but still.

metalwarrior665

I can see the need for this complexity as we have to handle Dataset.pushData that only connects to SDK via the client

metalwarrior665 · 2025-12-15T18:52:42Z

packages/apify/src/actor.ts

+                const chargeResult = await actor.chargingManager.charge({
+                    eventName,
+                    count: countToCharge,
+                });


We shouldn't be charging platform via API here. So maybe a new option to only increment locally or a new protected method?

janbuchar · 2025-12-17T09:35:18Z

We need to prevent users from (internally) charging the synthetic event manually (API will not accept it, so that part is already done)

…aset-events

janbuchar · 2025-12-19T21:23:05Z

This is about as readable as I can make it. We should add more tests and documentation (after holidays).

janbuchar · 2026-01-12T12:47:50Z

packages/apify/src/actor.ts

-            return await this.chargingManager.charge({
+        const dataset = await this.openDataset();
+
+        if (this.usesPushDataInterception(dataset)) {


Explain why we need to handle this in two separate code paths

janbuchar · 2026-01-12T12:48:07Z

packages/apify/src/actor.ts

+        return context.chargeResult;
+    }
+
+    private async pushDataViaDirectCharging(


The name could be improved here

janbuchar · 2026-01-12T12:49:01Z

packages/apify/src/patched_apify_client.ts

+    chargeResult?: ChargeResult;
+}
+
+// Shared context used by Actor.pushData() and PatchedDatasetClient.pushItems().


Better explain the purpose of the context (propagate eventName to pushItems calls, gather results of multiple pushItems calls that were triggered by a single pushData call)

mhamas

We discussed the changes in person. In general they look good to me, but since I don't understand the specifics of SDK, I cannot give very solid review. Leaving a couple of extra comments.

mhamas · 2026-01-09T16:35:37Z

packages/apify/src/charging.ts

    perEventPrices: Record<string, number>;
 }

+export function mergeChargeResults(


Suggestion: this is good candidate for unit tests

mhamas · 2026-01-12T12:50:11Z

packages/apify/src/actor.ts

+        const { limitedItems, eventsToCharge } =
+            this.chargingManager.calculatePushDataLimits({
+                items,
+                eventName: explicitEventName,
+                isDefaultDataset,
+            });
+
+        if (limitedItems.length > 0) {
+            // Preserve original `Dataset.pushData()` call shape for single items.
+            await dataset.pushData(
+                Array.isArray(items) ? limitedItems : limitedItems[0],
+            );
+        }


Suggestion: Looks duplicated to the intercepted client path. Can this be taken out and unified?

mhamas · 2026-01-12T12:52:15Z

packages/apify/src/charging.ts

+     * Helper to calculate how many items can be pushed within charging limits.
+     * Returns the limited items and count to charge.
+     */
+    calculatePushDataLimits<T>({


Suggestion: I'd add unit tests.

barjin · 2026-01-12T13:32:49Z

packages/apify/src/actor.ts

     * The objects must be serializable to JSON and the JSON representation of each object must be smaller than 9MB.
     * @ignore
     */
    async pushData(item: Data | Data[]): Promise<void>;


This is not relevant anymore, the signature on L1100 reflects the return type better

barjin

We've discussed the design with @janbuchar offline, IMO we should still drop the local total and fetch it somehow from the API. Regarding the implementation, I found two talking points:

barjin · 2026-01-12T14:28:29Z

packages/apify/src/actor.ts

+            if (explicitEventName !== undefined) {
+                return results[explicitEventName];
+            }
+        }
+
+        return undefined;


Actor.pushData() will charge the synthetic event, but will return undefined here.

See e.g. the following code, run with ACTOR_MAX_TOTAL_CHARGE_USD=2:

import { Actor } from 'apify'; process.env.ACTOR_TEST_PAY_PER_EVENT = '1'; await Actor.init(); console.log(await Actor.pushData([{ 'a': 1 }, { 'b': 2 }, { 'c': 3 }])); // prints undefined console.log(await Actor.pushData([{ 'a': 1 }, { 'b': 2 }, { 'c': 3 }])); // prints undefined console.log(await Actor.pushData([{ 'a': 1 }, { 'b': 2 }, { 'c': 3 }])); // prints undefined console.log(await Actor.pushData([{ 'a': 1 }, { 'b': 2 }, { 'c': 3 }])); // prints undefined await Actor.exit();

This will run all the calls, but will store only two results into the dataset. The developer has no way of returning early, as they won't get the charging manager response here.

barjin · 2026-01-12T14:32:11Z

packages/apify/src/actor.ts

+            await Promise.all(
+                Object.entries(eventsToCharge).map(
+                    async ([eventName, count]) => {
+                        results[eventName] = await this.chargingManager.charge({


calculatePushDataLimits will contain the apify-default-dataset-item key (see here). Here we'll manually charge the synthetic event. Is this expected?

janbuchar added 2 commits December 12, 2025 17:02

fix: Keep track of synthetic apify-default-dataset-item events

60ff30f

Handle non-Apify runs as well

ce37407

janbuchar added the t-tooling Issues with this label are in the ownership of the tooling team. label Dec 12, 2025

B4nan added t-tooling Issues with this label are in the ownership of the tooling team. and removed t-tooling Issues with this label are in the ownership of the tooling team. labels Dec 15, 2025

mhamas mentioned this pull request Dec 15, 2025

docs: ppr announced as legacy model, ppr to ppe migration apify/apify-docs#2144

Merged

metalwarrior665 reviewed Dec 16, 2025

View reviewed changes

janbuchar added 2 commits December 19, 2025 15:25

Merge remote-tracking branch 'origin/master' into track-synthetic-dat…

e19555c

…aset-events

Refactoring and general logic fixes

2df6efc

github-actions bot assigned janbuchar Dec 19, 2025

github-actions bot added this to the 130th sprint - Tooling team milestone Dec 19, 2025

github-actions bot added the tested Temporary label used only programatically for some analytics. label Dec 19, 2025

Restore original quirks

ac886af

janbuchar marked this pull request as ready for review December 19, 2025 21:23

janbuchar requested review from B4nan and barjin December 19, 2025 21:23

Fix single-item push data calls

4477dde

janbuchar requested review from metalwarrior665 and mhamas January 6, 2026 20:52

janbuchar commented Jan 12, 2026

View reviewed changes

mhamas reviewed Jan 12, 2026

View reviewed changes

barjin reviewed Jan 12, 2026

View reviewed changes

fix: Keep track of synthetic apify-default-dataset-item events #528

Are you sure you want to change the base?

fix: Keep track of synthetic apify-default-dataset-item events #528

Uh oh!

Conversation

janbuchar commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

janbuchar commented Dec 12, 2025

Uh oh!

janbuchar commented Dec 12, 2025

Uh oh!

janbuchar commented Dec 15, 2025

Uh oh!

metalwarrior665 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janbuchar commented Dec 17, 2025

Uh oh!

janbuchar commented Dec 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhamas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barjin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

janbuchar commented Dec 12, 2025 •

edited

Loading