Add support for user provided context in handlers

[srikrsna-buf]

Add support for user provided context in handlers. With this change handlers can depend on user provided values in the context.

Closes #818, #586, and #708

Related: #550

Example:
Create a context key with a default value:

export interface User {
  id: string;
}
import { createContextKey } from "@connectrpc/connect";
export const kUser = createContextKey<User | undefined>(
  undefined // The default value.
);

Use the contextValues option to provide the context values for each request:

import { fastify } from "fastify";
import routes from "./connect";
import { fastifyConnectPlugin } from "@connectrpc/connect-fastify";
import { authenticate } from "./authenticate.js"; 
import { kUser } from "./user-ctx.js";

const server = fastify();

await server.register(fastifyConnectPlugin, {
 routes,
 contextValues: (req) => createContextValues().set(kUser, authenticate(req)),
});

await server.listen({
  host: "localhost",
  port: 8080,
});

Use the context value in the handler:

import { ConnectRouter } from "@connectrpc/connect";
import { ElizaService } from "./gen/eliza_connect";

export default (router: ConnectRouter) => {
  // using const say
  router.service(ElizaService, {
    say: (req, { values }) => {
      const currentUser = values.get(kUser);
      if (currentUser === undefined) {
        throw new ConnectError("Unauthenticated", Code.Unauthenticated);
      }
      // ...
    },
  });
};

[paul-sachs]

I love the way this keeps the type information associated to the context id, very cool. One question I have is if we want to expose the context values to interceptors as well?

[srikrsna-buf]

I love the way this keeps the type information associated to the context id, very cool. One question I have is if we want to expose the context values to interceptors as well?

We don't have server side interceptors yet, but if we finalize this API I will send a follow up to add this to the clients. The server side interceptors should include them when implemented.

[smaye81]

This may be a contrived example, but what if the user wants to add something to context that doesn't involve the request object? I suppose they could do:

await server.register(fastifyConnectPlugin, {
 routes,
 contextValues: () => createContextValues().set(kUser, {"name": "Krishna"}),
});

Does it make sense then to make the req optional in the contextValues function property?

[srikrsna-buf]

@smaye81 Your example will work without any changes, req is something we pass to the function and can be omitted if they don't need it.

[timostamm]

I imagine one might want to console.log context values or inspect them in a debugger.

Should we improve behavior with an optional description string for the symbol, and by storing values as properties instead of a captured variable?

[jeffsu]

Would it be helpful to consider a solution that introduces typesafe constraints between the route handlers and the context builder?

Here is a rough example of the types I'm talking about:

// context now has typesafety
type Context<V={}> = { values: V };

// example of how a route handler can assert that it needs some data out of context
type RouterHandler<I, O, V={}> =  (input: I, context?: Context<V>) => O;

// then we can force the injection of some data in context
class Server<C extends Context> {
  addContextValue<K extends LiteralString, T>(lambda: (req: Request) => T): Server<Context<C["values"] & { K: T}>>;
}

[srikrsna-buf]

Thank you for the suggestion @jeffsu!

Could you give us a simple e2e example of how this will work? I am unable to see how this can guarantee the presence of a value. In the current design, users have to pass a default value.

One other thing to note is that we want to add context values to interceptors on both the client and server side (not implemented yet). Values can be added via the interceptors as well which means the values are not always available at the request start.

[srikrsna-buf]

Hey @jeffsu to illustrate what I meant by adding them to interceptors I just opened #841 please take a look and let me know what you think.

One thing I noticed with the current implementation is that ContextValues is mutable. Is immutability a desired property?

Current behavior:

const interceptor = (next) => {
     return (req) => {
          const user = req.values(kUser);
          const res = next(req); // This can modify req.values
          // req.values(kUser) !== user
     }
}

cc @timostamm @paul-sachs @smaye81

[paul-sachs]

This level of mutability doesn't really bother me. If we made a version of context that could be deep cloned, you could do something like:

const interceptor = (next) => {
     return (req) => {
          const user = req.values(kUser);
          const res = next({ ...req, values: req.values.clone() }); // This can modify req.values
          // req.values(kUser) === user
     }
}

[timostamm]

It's true that handlers cannot declare a dependency on a context value with this, as Jeff noted. I think this is a nice step forwards though. I say LGTM.

[mblode]

Is there an example of how to use this new system to get the logger inside the connect service?

E.g

fastify.get('/', options, function (request, reply) {
  request.log.info('Some info about the current request')
  reply.send({ hello: 'world' })
})

router.service(Health, {
  check(request, context) {
    // log here
    console.log(context)
  }
}

[srikrsna-buf]

Is there an example of how to use this new system to get the logger inside the connect service?

It is the same as the example in the PR description, instead of
contextValues: (req) => createContextValues().set(kUser, authenticate(req)),
you can just use a different key for the log type:
contextValues: (req) => createContextValues().set(kLog, req.log),

And then can get the value and use it:

router.service(Health, {
  check(request, context) {
    context.values.get(kLog).log.info(...);
    console.log(context)
  }
}

[StarpTech]

Why was the implementation shipped this way? We have no async support and we can't pass a global type for contextValues. Our use case is to check authentication in this handler. Do you accept a PR?

[srikrsna-buf]

@StarpTech Typically if you're using something like Fastify/Express you will have an auth library that does the authentication checks and adds the subject info to the incoming request, and you can add the that to the context.

we can't pass a global type for contextValues.

This is actually error prone and could have conflicts. Using a unique key lookup helps us guarantee type safety and always provide a default value.

[StarpTech]

I see your points but why should we limit the implementation to only synchronous work? I don't see the value in it.

This is actually error prone and could have conflicts. Using a unique key lookup helps us guarantee type safety and always provide a default value.

I'm quite on the opposite side. We have to specify a shared ContextKey Type and use the right type everywhere we access it instead of maintaining the relation once

This would be more conenvient (example, could be specified somewhere else)

(router: ConnectRouter) => {
    router.service<MyContextValuesType>(MyService, MyServiceServiceImpl(opts), handlerOptions);
  };

Access

 ctx.values.get(Symbol('foo')) // return the correct type, typesafe access out of the box

Another thing we find odd is, why is it necessary to specify the the default value when retrieving a value? If the value can be undefined, it is the dev responsibility to deal with it.

// Error: Property defaultValue is missing in type { id: symbol; } but required in type ContextKey<unknown>
ctx.values.get({
  id: Symbol('subgraph'),
});

[srikrsna-buf]

Access

 ctx.values.get(Symbol('foo')) // return the correct type, typesafe access out of the box

This is how it can be used today, instead of creating a Symbol the key is nothing but a type-safe proxy to a static symbol.

Another thing we find odd is, why is it necessary to specify the the default value when retrieving a value? If the value can be undefined, it is the dev responsibility to deal with it.

Default is not needed when retrieving a value, it is only needed when creating the key. If undefined is needed, then the type can be set to do that: T | undefined.

We just updated the docs with an example. Hopefully that helps clarify the usage a little bit. This is fairly a common pattern in the JS ecosystem, a popular mechanic that works like this is the React Context.

[StarpTech]

This is how it can be used today, instead of creating a Symbol the key is nothing but a type-safe proxy to a static symbol.

Was the API updated? This is not possible in 1.1.3.

Default is not needed when retrieving a value,

This doesn't work for me.

Property defaultValue is missing in type { id: symbol; } but required in type ContextKey<string | undefined>
 ctx.values.get<string | undefined>({
        id: Symbol('test'),
      });

We just updated the docs with an example.

There you made an example to authenticate within contextValues. Based on your previous suggestions, I think this should be removed.

[srikrsna-buf]

The API is the same, you have to create a unique key using createContextKey and use that. Symbol constructor creates a unique symbol and cannot be used like this:

console.log(Symbol('foo') === Symbol('foo'));
// Expected output: false

[StarpTech]

Ah, ok createContextKey is just a utility function. I think I know now where my confusion comes from. The API is just not very common in the Node.js space. Thanks again.