Cannot use middleware and asynchronous service method

[Cheerazar]

With the current implementation of compose-middleware.ts, I cannot seem to get it to work properly with the getCat example if I add a middleware. It doesn't matter if that middleware is synchronous or asynchronous.

import { ProtoCat } from 'protocat'
import { CatRegisterService } from './cat_grpc_pb'
const app = new ProtoCat()

app.use((call, next) => {
  console.log('First middleware')
  next()
})
app.use((call, next) => {
  console.log('Second middleware')
  next()
})

const findCat = name =>
  new Promise(resolve =>
    setTimeout(() => {
      resolve({
        name,
        level: Math.round(Math.random() * 10),
        health: Math.random() * 100,
      })
    }, 3000)
  )

export async function getCat(call) {
  try {
    const cat = await findCat(call.request.getName())
    call.response.setName(cat.name).setLevel(cat.level).setHealth(cat.health)
  } catch (error) {
    console.error('Error in getCat', error)
  }
}

app.addService(CatRegisterService, {
  getCat,
})

app.start('0.0.0.0:3000')

grpcurl \
    -proto cat.proto \
    -plaintext \
    -d '{"name": "ProtoCat"}' \
    localhost:3000 cats.v1.CatRegister.GetCat

syntax = "proto3";
package cats.v1;
service CatRegister {
  rpc GetCat(GetCatRequest) returns (Cat) {};
}
message Cat {
  string name = 1;
  float health = 2;
  int64 level = 3;
}
message GetCatRequest { string name = 1; }

When I do the grpcurl above, I get an empty response back. I believe that the reason for this is that in compose-middleware.ts, you do await dispatch(0), and in application.ts you await methodHandler(call). As soon as the dispatch(0) resolves, the call in application.ts will resolve, even though there could be additional middlewares and the final call to resolve.

Have I missed something for how to get this to work correctly or is there a bug here?

I was able to get the middleware to work nicely and to complete in the order I expected by using a for loop and awaiting each dispatch to complete. When I did this, it didn't pass the tests and didn't operate exactly as I think you had intended.

compose-middleware.ts

import { Middleware } from '../call'
/**
 * Compose a list of Middlewares into one, connecting the next function stack
 * @internal
 */
export const composeMiddleware = (middleware: Middleware[]): Middleware => {
  const mwLength = middleware.length
  return async (call, next) => {
    const dispatch = async (i: number): Promise<void> => {
      // TODO: Maybe throw / log on repeated next
      const fn = i === mwLength ? next : middleware[i]
      if (!fn) return
      await fn(call, () => Promise.resolve())
    }
    for (let i = 0; i < mwLength; i += 1) {
      try {
        await dispatch(i)
      } catch (error) {
        throw error
      }
    }
  }
}

[smolijar]

Hi, thanks for report.

As soon as the dispatch(0) resolves, the call in application.ts will resolve, even though there could be additional middlewares and the final call to resolve.

dispatch(0) is the first middleware, dispatch(1) second, ..., dispatch(n-1) another and dispatch(n) the service handler in usual scenario. So middlewares are all called and awaited correctly.

Your suggestions awaits middlewares one by one, disabling the use of next cascade (awaiting the callstack and doing something after), that's why tests fail.

The problem is in your middlewares, you do not return/await the next call. Correct solution would be e.g.:

app.use((call, next) => {
  console.log('First middleware')
  return next()
})
app.use(async (call, next) => {
  console.log('Second middleware')
  await next()
})

Unless you want to end the execution, your middlewares must return a promise from next() fn (or be async and await next() ofc). What happened is that your first middleware was correctly awaited, but all middlewares/handlers stack behind the next function could not be. Implicit response was returned, since the first middlewares (the only one that finished) did not touch the response object.

Please read the docs on the next function, there's a warning about it https://proto.cat/docs/wiki/middlewares#next-function. If you read it, and it is unclear from the that, feel free to publish a PR to improve it 🤗

[Cheerazar]

The problem is in your middlewares, you do not return/await the next call

🤦 That was the exact problem. I figured that I was missing something simple. I read that section that you linked, but I must have glossed over the big warning. Thank you!