Wrapper methods in GRPCClientClient to get parameter types of the wrapped GRPC call

[tegefaulkes]

Specification

Here is an example of a function from src/client/GRPCClientClient.ts:39:

  public vaultsCreate(...args) {
    if (!this._started) throw new clientErrors.ErrorClientClientNotStarted();
    return grpcUtils.promisifyUnaryCall<clientPB.StatusMessage>(
      this.client,
      this.client.vaultsCreate,
    )(...args);
  }

While the return type is explicit here as grpcUtils.promisifyUnaryCall<clientPB.StatusMessage>.
The type information for the parameters are completely missing. What do I pass to this? is it one parameter or many?
To find out I need to go look at the definition of this.client.vaultsCreate

public vaultsCreate(request: Client_pb.VaultMessage, metadata: grpc.Metadata, options: Partial<grpc.CallOptions>, callback: (error: grpc.ServiceError | null, response: Client_pb.StatusMessage) => void): grpc.ClientUnaryCall;

We should explicitly state the parameters and their types like so:

  public vaultsCreate(vaultMessage: clientPB.VaultMessage):Promise<clientPB.StatusMessage> {
    if (!this._started) throw new clientErrors.ErrorClientClientNotStarted();
    return grpcUtils.promisifyUnaryCall<clientPB.StatusMessage>(
      this.client,
      this.client.vaultsCreate,
    )(vaultMessage);
  }

Additional context

This is used by the CLI and the Polykey GUI to handle comunication to the agent. it will be used extensively. adding type information will make it:

• easier to write code for.
• proper compile time type checking by the typescript compiler.

As it curretly is error will only come up during runtime when calling the function.

Tasks

1. Replace ...args with the respective parameter and type for each function.

[CMCDragonkai]

@tegefaulkes can you rewrite this issue with the new issue templates? Is this a design issue or development issue? It cannot be both.

[tegefaulkes]

There still are no templates when creating a new issue. I took this one from the .github development template.
The table at the top doesn't seem to be working. Also, why is there a name and title in it?

[CMCDragonkai]

That doesn't happen for me. Go to the GitHub issues and click new issue. You'll see it.

…

On 6 July 2021 11:21:56 am AEST, Brian Botha ***@***.***> wrote: There still are no templates when creating a new issue. I took this one from the .github development template. The table at the top doesn't seem to be working. Also, why is there a name and title in it? -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #200 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[CMCDragonkai]

@tegefaulkes this missing type information is by design. The reason is that GRPCClientClient.vaultsCreate is a simple wrapper the actual call.

If we do what you are suggesting, it would require us to update the types twice everytime we changed the protobuf calls. So this is why we used the ...args.

There is a possible solution using the utility types of typescript to make the types propagate properly: https://www.typescriptlang.org/docs/handbook/utility-types.html#parameterstype

However I think this is not high priority unless you can figure out which one to use quickly.

[CMCDragonkai]

Here's an example:

function f (n: number) {
  return n;
}

type FParams = Parameters<typeof f>;

Suppose we use:

type FParams = Parameters<typeof this.client.vaultsCreate>;

The problem is that this.client.vaultsCreate is a callback style. And we would have to ignore the last argument.

Another problem is that this.client.vaultsCreate is also overloaded as well. So I think Parameters might work as a union of the all styles. But you then have to somehow ignore the last argument.

I don't know how to slice off the last type. So for now, we'll keep this issue in the back burner.

[CMCDragonkai]

Looks like there is a way: https://stackoverflow.com/questions/63789897/typescript-remove-last-element-from-parameters-tuple-currying-away-last-argum

Basically we just need the remove the last parameter which is the callback.

[CMCDragonkai]

We would have to create our own utility type here in src/types.ts for that. Something like ParametersExceptLast.

Note that this would have to be different for the stream calls which have different signatures. Would need to check how the protobuf works.

[CMCDragonkai]

Relevant to #230. @DrFacepalm I wonder if this would actually prevent us from making those signature mistakes when calling the grpc stub functions.

[CMCDragonkai]

@DrFacepalm has achieved this on post_session-and-misc_fixes branch.

We'll be using a type Initial and type InitialParameters since I discovered that haskell calls the parameters except last "init". So Head vs Tail and Initial vs Last is the correct terminology.

[CMCDragonkai]

@tegefaulkes This does also fix #230? Or will that require some changes to the tests?

[tegefaulkes]

I've been looking into this and I've looked over what @DrFacepalm has done. The utility types works to a degree, but there are 2 complications with it. It seems to be pretty hard to infer the return type from the callback, I haven't found something that works for that yet.

The other problem is that utility types doesn't support overloaded function definitions currently and it doesn't look like that feature will be added.
microsoft/TypeScript#26591

So while yes, we can get the all the arguments minus the callback and infer that properly. the result is we're limited to just one of the overloaded function definitions.

So at this stage I don't think there is an easy solution for this.

[CMCDragonkai]

The last 3 comments here microsoft/TypeScript#32164 (comment) may provide a hack for this. Can resolve this later.

[tegefaulkes]

Fixes to #230 has removed the need to provide metadata and call options for most calls now. This has become low priority now. Moving it back to ToDo

[CMCDragonkai]

The lack of type signature propagation led to this problem: https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/213#note_739516636. It basically involved the usage of metadata parameter.

Note that propagating the type signatures can be important all the way to the CARL retry functions if we make use of the automatic way of passing metadata or not.

retryUnary(
  grpcClient.sessionsUnlock.bind(grpcClient),
  [passwordMessage],
  metaInitial
);

It's going to require some special types to propagate this. The second parameter array must be matched up with the arguments of the lower order function, but with possibly an additional metadata. One might even need to pass relevant call options too?

[CMCDragonkai]

Back to do, we resolved this for now just by always using a common idiom in for retryAuthentication.

[CMCDragonkai]

Once we moved to JSON RPC, we no longer have this problem because we won't be using any code generation.

[tegefaulkes]

That makes this Issue irrelevant now. I think we should close it.