Skip to content
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

WIP Core logic refactor #182

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

WIP Core logic refactor #182

wants to merge 10 commits into from

Conversation

evanderkoogh
Copy link
Owner

Very much Work In Progress and not properly tested yet.. do not merge.

Fixes #150
Fixes #179

What this PR solves / how to test:

This PR is a major rewrite of the core logic.. right now there is a lot of duplication in the individual instrumentations around handling configuration, instrumentation of Env and ExecutionContext etc etc.. But because everything was extremely, but not quite the same, it was hard to abstract out.

But now that we have a few different implementations it became clear how to abstract it properly and that is what this PR is. It greatly reduces the surface areas for subtle bugs like Promise not completing to hide.

@evanderkoogh evanderkoogh self-assigned this Dec 2, 2024
@evanderkoogh evanderkoogh marked this pull request as draft December 2, 2024 04:04
@supfilistine
Copy link

Any desire to include support for instrumenting Rpc classes? I feel like DOs are very similar when not orchestrated around having a fetch handler, thus themselves having an Rpc interface.

I think the cloudflare types just end up calling these the WorkerEntrypoint interfaces (when not a DO worker); not sure if it's totally introspectable from the perspective of implementing a proxy for each of the public methods - not caught up on all the TS advancements. My approach with current Rpc class based workers is to have public and private methods and public ones are the ones I invoke from other workers.

If instrumentation can't detect them easily, I don't mind if the instrument method took a specific list of methods as an argument.

WorkerEntrypoint classes do take an Env on the constructor, so it would be great if the passed bindings were automatically instrumented.

Just throwing this out as something that I very much desire from this well written library! Happy to help test these changes on my new project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants