refactor(gteam-internal): delegate safePushData to pushDataWithSchemaRepair - #13
Conversation
…Repair Bad items are now repaired or dropped-and-logged instead of throwing NonRetryableError and failing the whole push. Drops the now-unused @crawlee/core dependency along with the hand-rolled AJV error handling it existed for.
JuanGalilea
left a comment
JuanGalilea
left a comment
There was a problem hiding this comment.
i mean its your thing, but at this point i don't see the point of this function tbh.
The whole purpose i see is the not having to pass in Actor.pushData or Dataset.pushData.
| const msg = 'Dataset validation failed'; | ||
| const { droppedItems, pushResult } = await pushDataWithSchemaRepair(pushFn, data); | ||
| for (const { errors } of droppedItems) { | ||
| log.error(msg, { msg, validationErrors: errors }); |
There was a problem hiding this comment.
pushDataWithSchemaRepair already logs stuff btw 👀
There was a problem hiding this comment.
We thought about adding our own logs on top of the ones that are already present to keep our existing monitoring flow
| if (!alias) { | ||
| if (eventName) { | ||
| return wrapPushData(async () => Actor.pushData(data, eventName)); | ||
| const pushResult = await wrapPushData(data, async (items) => Actor.pushData(items, eventName)); |
There was a problem hiding this comment.
we should have this as a feature of the main thing actually 👀
There was a problem hiding this comment.
If we never successfully push anything, we don't get back any pushResult to return to the caller. Then it is really up to the team how they want to manage it, we should give empty charge result out of nowhere
Or did you mean something else?
4 participants
Why?
We had our old implementation in the gteam-internal sub-package, and now we need to replace it with the schema-repairing implementation from the main package.
What?
safePushData now delegates to pushDataWithSchemaRepair for all three call shapes. Every dropped item is still logged through log.error the same way as before, so existing log-based monitoring keeps working, but the push itself resolves instead of throwing. A synthetic charge-less ChargeResult is returned for the eventName case when every item ends up dropped and nothing was actually pushed or charged.
The now-unused @crawlee/core dependency was removed from package.json (dependencies, peerDependencies, peerDependenciesMeta) along with its README mention, since NonRetryableError was the only thing that ever needed it here.
Further notes:
The tests were updated to mock the exact error shape pushDataWithSchemaRepair's isSchemaValidationError actually checks for (type, statusCode, data.invalidItems), matching what apify-client reports on a real schema-validation 400, and now assert the drop-and-log-and-continue behavior instead of the old throw.