Arwalk / zig-protobuf

a protobuf 3 implementation for zig.
MIT License
188 stars 20 forks source link

Generate services interfaces #40

Open Arwalk opened 4 months ago

Arwalk commented 4 months ago

closes #39

Arwalk commented 4 months ago

I've tried adding some cool comptime checks on the init method based on reflection of functions and all, but the truth is that they never get hit by the compiler. If a method doesn't exist, or misses a parameter, or doesn't have a right type somewhere, the compiler detects it before any of my checks.

So this should be enough to generate services interfaces.

Arwalk commented 4 months ago

my brain is smooth, i forgot about error handling on the interfaces.

menduz commented 4 months ago

I think we should get first a library that does RPC and then create the implementation on top of it. The PR may include a good approach, but I'd hesitate on committing to it without a real-world use case.

Besides that, there are three concise things we should discuss:

  1. More often than not, implementations of methods return Future|Promise|AsyncHandles. We may need to converge towards some managed structure that holds a value, a status and a callback. Future(T) = struct { resolved: bool, value: ?T, callback: fn(error, T), fn deinit() }
  2. Both streaming requests and responses are not yet implemented but are critical to grpc
  3. We should think about implementing remote, transport and runtime errors
Arwalk commented 4 months ago

I think we should get first a library that does RPC and then create the implementation on top of it. The PR may include a good approach, but I'd hesitate on committing to it without a real-world use case.

Besides that, there are three concise things we should discuss:

  1. More often than not, implementations of methods return Future|Promise|AsyncHandles. We may need to converge towards some managed structure that holds a value, a status and a callback. Future(T) = struct { resolved: bool, value: ?T, callback: fn(error, T), fn deinit() }
  2. Both streaming requests and responses are not yet implemented but are critical to grpc
  3. We should think about implementing remote, transport and runtime errors

Rpc services defined in protobuf are not necessarily gRPC, and i do not plan on implementing gRPC in this repository. If i ever tackle the subject, it would be on another repository, build on top of this one. This PR's goal is to have at least some naive interface out of defined services, and end users are free to branch it to whatever protocol they prefer.

For async, coroutines are sadly still not back on the menu in the compiler, so i'd prefer to wait a stable release on this before trying to half-bake something that we'd have to maintain inside.

Error handling is discussed in the original thread, i plan on giving end users this, but implementations will obviously have to handle their own errors somehow.

I kinda agree with what you said, but it assumes all protobuf services uses gRPC and that's not the case according to the spec itself.

menduz commented 4 months ago

Rpc services defined in protobuf are not necessarily gRPC

I completely agree, and personally spent several months working on a protobuf-based RPC mechanism that is not gRPC either https://github.com/decentraland/rpc.

My argument on this matter is that we should provide some mechanism to enable wrapping responses and stream request/responses to enable async capabilities. I cannot think about one single case of RPC that is not asynchronous (outside of microcontrollers with interrupts), for everything else keep plain old structs.

In pseudocode:

-pub const TestService = struct {
+fn TestService(comptime AsyncHandle: fn (anytype) type) type {
+ return struct {
    const Interface = struct {
-        Foo: *const fn (x: *anyopaque, p: FooRequest) FooResponse,
+        Foo: *const fn (x: *anyopaque, p: FooRequest) AsyncHandle(FooResponse),
    };

    _ptr: *anyopaque,
    _vtab: *const Interface,

    pub fn init(obj: anytype) TestService {
        const Ptr = @TypeOf(obj);
        const PtrInfo = @typeInfo(Ptr);
        std.debug.assert(PtrInfo == .Pointer); // Must be a pointer
        std.debug.assert(PtrInfo.Pointer.size == .One); // Must be a single-item pointer
        std.debug.assert(@typeInfo(PtrInfo.Pointer.child) == .Struct); // Must point to a struct
        const impl = struct {
-            pub fn Foo(ptr: *anyopaque, param: FooRequest) FooResponse {
+            pub fn Foo(ptr: *anyopaque, param: FooRequest) AsyncHandle(FooResponse) {
                const self: Ptr = @ptrCast(@alignCast(ptr));
                return self.Foo(param);
            }
        };
        return .{ ._ptr = obj, ._vtab = &.{
            .Foo = impl.Foo,
            .Bar = impl.Bar,
        } };
    }

-    pub fn Foo(self: TestService, param: FooRequest) FooResponse {
+    pub fn Foo(self: TestService, param: FooRequest) AsyncHandle(FooResponse) {
        return self._vtab.Foo(self._ptr, param);
    }
};

Another option is to specify that in the code generator, for project-idiomatic code generation

// build.zig
 const protoc_step = protobuf.RunProtocStep.create(b, protobuf_dep.builder, target, .{
    .services = .{
      .async_wrapper_name = "AsyncWrapper",
      .async_wrapper_import = "some-lib",
     // generates something like `const AsyncWrapper = @import("some-lib");`
    },
});

A third option, and the one that reduces all uncertainty, is to expose services metadata as comptime const structs, like we do for fields.

Those can be traversed and used to generate code and to perform validations in a per-project basis.

// myService.pb.zig

...

pub const MyService = struct {
  const methods = .{
    const Foo = .{
      .request_type = FooRequest,
      .request_is_streaming = false,

      .response_type = FooResponse,
      .response_is_streaming = false,
    };
  };
};
Arwalk commented 4 months ago

Yeah, i see you're point. I'm gonna look a bit more into it and how it's done in google's C++ interfaces for services. But i'm really bothered by the fact that async is currently not available in zig. I can't find a Future/Promise/AsyncResult in the std right now.

That said, i found https://github.com/Jack-Ji/zig-async which could be an inspiration (i don't agree with some of the approaches on it and i would really like to avoid relying on another library).

In the meantime, I still think a simple and naive synchronous interface could be useful somehow. I'll think a bit more about it.

menduz commented 4 months ago

I can't find a Future/Promise/AsyncResult in the std right now.

https://github.com/Jack-Ji/zig-async/blob/main/src/task.zig#L7 is perfect! Exactly as I was thinking about it.

I'd say that we can pass in Future as comptime parameter to generate the service type.

// generated code
fn MyService(comptime ResponseWrapper: type) type {
    return struct {
      // ptr
      // vtable

      fn Foo(req: Request) -> ResponseWrapper(Response) {...}
    };
}

// user code for async response types using ZigFuture
const MyServiceAsync = pb.MyService(Future);

// user code for synchronous responses
const MyServiceSync = pb.MyService(Identity);
fn Identity(comptime T: type) type {
    return T;
}

I still think a simple and naive synchronous interface could be useful somehow

I'd be all in on this approach if we can find those useful cases. Did you think about any?

Arwalk commented 3 months ago

Very late answer, have been pretty busy lately

I'd be all in on this approach if we can find those useful cases. Did you think about any?

As you said, there are the use case of simple devices with no real use for asynchronous patterns. Also i think it could be nice for server-side implementations to have it?

But it's kind of annoying to generate 2 different interfaces for the same thing, so i guess it makes more sens to work on a Future(T) interface, and propose a blocking version that should be easy to use for test and less evolved hardware/systems.

I guess i still have to think a bit more about it.