Initial implementation of DO-attached containers API #3354
Conversation
No methods are implemented yet, but if a container capability is passed in when creating an actor, then `ctx.container` will be populated.
Tests are currently in the internal codebase, since that's where the mock container service lives, but probably some of that should be ported out here later, especially once we figure out how local testing of containers will work.
|
The generated output of Full Type Diff |
| @@ -536,6 +543,7 @@ class DurableObjectState: public jsg::Object { | |||
| JSG_METHOD(waitUntil); | |||
| JSG_READONLY_INSTANCE_PROPERTY(id, getId); | |||
| JSG_READONLY_INSTANCE_PROPERTY(storage, getStorage); | |||
| JSG_READONLY_INSTANCE_PROPERTY(container, getContainer); | |||
There was a problem hiding this comment.
Nit: these could probably be JSG_LAZY_READONLY_INSTANCE_PROPERTY
|
|
||
| class Fetcher; | ||
|
|
||
| class Container: public jsg::Object { |
There was a problem hiding this comment.
Can you add code comments explaining what a Container is
|
|
||
| void visitForGc(jsg::GcVisitor& visitor) { | ||
| visitor.visit(destroyReason); | ||
| } |
There was a problem hiding this comment.
Nit: implement visitForMemoryInfo(...) for the destroyReason field.
| } | ||
| req.setEnableInternet(options.enableInternet); | ||
|
|
||
| IoContext::current().addTask(req.send().ignoreResult()); |
There was a problem hiding this comment.
Nit: Might be just a slight bit better to auto& ioContext = IoContext::current() before the auto req = rpcClient->startRequest() just in case the IoContext::current() fails. Not crtical and definitely an edge case tho.
|
|
||
| return IoContext::current() | ||
| .awaitIo(js, rpcClient->monitorRequest(capnp::MessageSize{4, 0}).send().ignoreResult()) | ||
| .then(js, [this](jsg::Lock& js) { |
There was a problem hiding this comment.
safer to capture self=JSG_THIS rather than this? It's unlikely that the container will disappear while this is pending but likely best to be safe?
| } | ||
|
|
||
| jsg::Promise<void> Container::destroy(jsg::Lock& js, jsg::Optional<jsg::Value> error) { | ||
| if (!running) return js.resolvedPromise(); |
There was a problem hiding this comment.
If destroy is called multiple times it looks like this will end up sending the destroyRequest multiple times? Also, it ends up creating and returning multiple promises. Once it has been called, it likely should be arranged to return the same js promise instance back.
| } | ||
|
|
||
| jsg::Promise<void> Container::monitor(jsg::Lock& js) { | ||
| JSG_REQUIRE(running, Error, "monitor() cannot be called on a container that is not running."); |
There was a problem hiding this comment.
What, if anything, should happen if this is called after "destroy()" is called but the destroy promise is still pending?
| } | ||
|
|
||
| void Container::signal(jsg::Lock& js, int signo) { | ||
| JSG_REQUIRE(signo > 0 && signo <= 64, TypeError, "Invalid signal number."); |
| running = false; | ||
| KJ_IF_SOME(d, destroyReason) { | ||
| jsg::Value error = kj::mv(d); | ||
| destroyReason = kj::none; |
There was a problem hiding this comment.
Non-blocking: this pattern still bugs. It's so easy for forget to set the var to kj::none after moving.
Implemented here:
Not implemented yet:
listenTcp()).