This repository has been archived by the owner on Nov 4, 2021. It is now read-only.
Babel Loop Timeouts #155
Merged
Babel Loop Timeouts #155
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… see if GH actions run
|
Had some trouble with tests in the end, but good to go. I added some extra metrics in the end as well. |
Twixes
approved these changes
Feb 18, 2021
|
There's one flaky test where it expects one number to be 32, but sometimes it's 33 or 34. So for now I'll just rerun the tests and hope they pass :). |
This was referenced Feb 18, 2021
Closed
fuziontech
pushed a commit
to PostHog/posthog
that referenced
this pull request
Oct 12, 2021
* transpile code via babel-standalone * move vm.ts into vm/ * add TASK_TIMEOUT config * add babel transform to timeout while loops * update refactored rename * vm while timeout test * add more unimplemented timeout tests * add transforms for while/for/do-while loops * fix loop timeout ms * remove extra ; * fix import * fix more imports * fix even more imports * fix error messages * simplify errors * small refactor * fix import * add async guard and protect against long promises * async guard around setupPlugin() * add column * slightly safer variables * less noise in vm bench * can not override async guard for the main functions (e.g. processEvent, etc) * reduce how much we do in a benchmark * add types to loop timeout * types for promise timeout * fix line/column numbers * explain some decisions * verify that the process event timeout applies e2e. * managed to get equal, so changing * update message that it's just a warning * add e2e kafka test for bad delay * shorter test in github action * increase test timeout to see if that makes a difference (locally it takes 3min for both tests) * skip the "slow on GH action" test for now * process kafka events in parallel by default * add more metrics to kafka queue * some debug to help fix the test * add debug log in the delayUntilEventIngested function * add "single_event_batch" timing to the event processing steps * fix timer * Improve timeoutGuard default timeout * revert benchmark to the last one that worked * skip bad delay * add back a 1:1 copy of the e2e.kafka test, but with the timeout code. see if GH actions run * remove broken tests, improve logging of working tests * Clan up vm.ts * Improve clarity of loopTimeout * Refactor transforms slightly * Refactor transform call to secureCode func * Add secureCode tests Co-authored-by: Michael Matloka <dev@twixes.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
@babel/standalone. This unlocks cool stuff like native TS support in the future. We might also look into caching the processed code.for,whileanddo whileloops, as described in this article using a babel plugin modified from one published by jsbin. Basically we'll check at the beginning of the loop and throw an error if it has been running too long.This greatly decreases our risk against some of the attacks described in #57 . This does not yet protect us against
long running promises nor"out of memory" scenarios. These could be built in to this PR or done later with another PR, we'll see how it goes.This might also have some implications and might need to be dialled down for scheduled plugins like
runEveryHour, where you might actually want a long running for loop.Checklist