timostamm / protobuf-ts

Protobuf and RPC for TypeScript
Apache License 2.0
1.1k stars 130 forks source link

Stream implementing Promise breaks returns in async #163

Open bluskript opened 3 years ago

bluskript commented 3 years ago

I am attempting to return a stream from an async function with the following code:

export const getStream = async (
  host: string,
  session?: string
): Promise<ChatStream | undefined> => {
  if (chatStreams[host]) return chatStreams[host];
  const conn = await getOrFederate(host);
  if (session) {
    conn.setSession(session);
  }
  const stream = conn.chat.streamEvents();
  chatStreams[host] = stream;
  stream.responses.onMessage((ev) => pubsub(host));
  stream.responses.onComplete(closeStreamHandler(stream));
  return stream;
};

Unfortunately, because DuplexStreamingCall implements Promise, usage of this function causes an await to result in FinishedDuplexStreamingCall, and closing the stream. I tried many ways to work around this issue, but it seems async resolves nested promises automatically. I had to settle with this hack:

export const getStream = async (
  host: string,
  session?: string
): Promise<{ boxed: ChatStream | undefined }> => {
  if (chatStreams[host]) return { boxed: chatStreams[host] };
  const conn = await getOrFederate(host);
  if (session) {
    conn.setSession(session);
  }
  const stream = conn.chat.streamEvents();
  chatStreams[host] = stream;
  stream.responses.onMessage((ev) => pubsub(host));
  stream.responses.onComplete(closeStreamHandler(stream));
  return { boxed: stream };
};
timostamm commented 3 years ago

Async functions are syntactic sugar for the Promise API. Returning a promise from an async function is the same as returning a promise in a Promise.then() callback - it will be resolved recursively. Unexpected behavior in my book, but that's what it is.

I am going to need more user feedback to go ahead here.

I've personally found calls being promise-like to be very convenient for unary methods, but cannot remember having really used it for other method types.

The consistency between the types is nice, but if there are no use cases for it and it has unwanted side-effects, it may be worth it to remove promise-like from all but the unary call type.