-
Notifications
You must be signed in to change notification settings - Fork 392
Implement setTimeout/clearTimeout in Maestro's JavaScript Environment #2254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Implement setTimeout/clearTimeout in Maestro's JavaScript Environment #2254
Conversation
@@ -7,6 +7,7 @@ import org.graalvm.polyglot.Context | |||
import org.graalvm.polyglot.Source | |||
import org.graalvm.polyglot.Value | |||
import org.graalvm.polyglot.proxy.ProxyObject | |||
import org.graalvm.polyglot.proxy.ProxyExecutable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used?
fun waitForActiveTimers(timeout: Long = 30_000) { | ||
if (activeTimersCount.get() > 0) { | ||
activeTimers.await(timeout, TimeUnit.MILLISECONDS) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this worth an integration test, that a 31s setTimeout never fires? I hate the idea of long running tests, but I can't think of a better way to test that this doesn't leave long running activities.
I'm not so worried about local running - I'm thinking about whether this change could provide any risk to the Maestro hosted infrastructure by a malicious or careless user.
Hi @Fishbowler I'll take a look on these comments on the next weekend. I was on vacations, sorry my delay |
Proposed changes
This PR implements the standard JavaScript timer functions (
setTimeout
/clearTimeout
) in Maestro's GraalJS environment, which previously didn't include these functions.Changes
GraalJsTimer.kt
GraalJsTimer
class to handle JavaScript timer functionalityAtomicInteger
CountDownLatch
GraalJsEngine.kt
setTimeout
andclearTimeout
functionsOrchestra.kt
Implementation Details
The implementation follows JavaScript's standard timer behavior:
setTimeout(callback, delay, ...args)
returns a numeric timer IDclearTimeout(timeoutId)
cancels a pending timeoutWhy This Matters
This implementation aligns Maestro's JavaScript environment with the JavaScript standard ecosystem, enabling essential asynchronous patterns in Maestro scripting. While the immediate use case involves polling mechanisms, it unlocks numerous scenarios including:
Testing
Tested with various scenarios: