The current definition for a Handler is:
export type Handler<TEvent = any, TResult = any> = (
event: TEvent,
context: Context,
callback: Callback<TResult>,
) => void | Promise<TResult>
Which correctly notes that handlers can return a promise, but the third argument for Callback is not required if the handler is an Async handler.
Shouldn't this argument be optional?
@types/xxxx package and had problems.Definitions by: in index.d.ts) so they can respond.No, because an optional argument in typescript means it's optional for the caller, because the implementing function must handle it not being present.
AWS is the caller, and they always provide it, so you, implementing the handler, don't need to handle it not being present.
If you don't want to do anything with an argument, you don't need to request it, e.g.
export const handler: AWSLambda.S3Handler = async event => {
...
};
@simonbuchan Totally understand, however, how does this work when trying to call the handler via a unit test? In that case, the unit test is the caller and the underlying handler is async?
The same as any other situation where you're testing an API that only needs part of an existing interface:
Export the actual interface you use (pretty easy to do implicitly now with things like as const), and get typescript to check that that's assignable to the actual interface elsewhere:
// for tests
export const actualHandler = async (event: S3Event): Promise<S3Result> => ...
// for type check
export const handler: S3Handler = actualHandler;
or:
// handler.ts
export const handler = async (event: S3Event): Promise<S3Result> => ...
// handler.test.ts
import { handler } from "./handler";
// check type
const s3Handler: S3Handler = handler;
...
Cast in the unit test:
function unused<T>() { return undefined as any as T; }
test("foo", async () => {
expect(handler("abc", unused<Context>(), unused<S3Result>()).to...
});
Actually test that they are not used:
const unusable: any = new Proxy(() => {}, {
get(target, name) {
throw new Error(`Invalid get of: ${String(name)}`);
},
set(target, name) {
throw new Error(`Invalid set of: ${String(name)}`);
},
apply() {
throw new Error(`Invalid call`);
},
});
test("foo", async () => {
expect(handler("abc", unusable, unusable);
});
and so on. None of this is specific to this API.
@simonbuchan I understand your reasoning, but the casting solution sucks.
That said, I can see AWS cannot modify their API because of backwards compatibility, and the Handler type must reflect that.
On the other hand, what I want - and probably what @duro wants as well - is to be able to say "I do not want the callback, my handler is using the async API". What I did in my project is to declare an AsyncHandler as follows:
export type AsyncHandler<TEvent, TResult> = (
event: TEvent,
context: Context
) => Promise<TResult>;
Then you can use declare your lambda with it:
export const handler: AsyncHandler<OrderRequest, APIGatewayProxyResult> = async (event) => {
// do stuff
}
This type is compatible with Handler and does not have the issue you mentioned. I simply declare I do not want the callback, and I guarantee I'm returning a Promise. My function then does not have to bother validation optional parameters, because the parameter is not there at all. Furthermore, if I really need the callback (e.g. ApolloServer project uses it), I can stick to the full Handler definition.
What do you think, does it make sense to stick the above AsyncHandler next to the existing Handler and let people choose which to use?
If you don't like casting, you can use any of the other solutions, or any other option you might think of if this were nothing to do with this package: off the top of my head, passing a jest.fn() that you later assert is not called, then wrap up that and calling the handler in another function if you don't want to have that code repeated. My point was that "I don't want to mock the whole interface that the function would be called with" is not the problem of this package.
But lets dismiss that for now to answer your question, adding AsyncHandler is very low effort yes. But it's also very low effort, and more importantly hard to get wrong for the user of this package, so there's a very low benefit to adding it. The aws-lambda package has value in:
OrderRequest HTTP body as the event type when you're using API Gateway, when it will be APIGatewayProxyEvent)Of the three, by far the most important is the first, but you're not blocked there: but the second is pretty trivial, at best saving you the slightly ugly node callback interface typing. It's pretty much just there for the third. So you wouldn't just need AsyncHandler, you would also need AsyncAPIGatewayProxyHandler, etc.: a much higher burden. Even then, that might make sense if the old callback could be deprecated and moved off of, but as you mentioned, it still is used and it really sucks to have incompatible interfaces when they have no reason to be so.
But the worst sin, is that it's a footgun:
// Perfectly valid typescript: `testableHandler` doesn't need that third parameter, and this is validating the event and result types!
export const handler: AsyncHandler<MyEvent, MyResult> = testableHandler;
export async function testableHandler(event: MyEvent, context: Context, apis = defaultApis()): Promise<MyResult> {
const someResult = await apis.getSomeValue(...);
return ...;
}
// handler.test.ts
test("some handler", async () => {
expect(await testableHandler(mockEvent, mockContext, mockApis)).toBe(expected);
A pity that this will blow up in actual Lambda, as the apis parameter will receive the callback argument (an example of how typescript isn't actually fully safe: this case should require AsyncHandler to take something like ...args: never[] to promise that it won't accept any more args).
Finally, this wouldn't really be a "use this if you are sure" things, as every handler is async, so everyone would assume that they should be using it, when it really only is for "if you want to make testing your handlers a bit easier when you are not fully simulating the way they will actually be called".
So, in summary, I really don't think this makes sense. If you're programming in Typsecript, you will occasionally run into type weirdness you aren't used to: this is one of those cases.
@simonbuchan
I think the footgun example is a bit contrived, you're showing that if you're looking for trouble, you're getting it :). There are many situations where typescript won't save you from yourself.
Note that it's not just about the extra callback argument; I was also looking to get rid of the void in the return type. Jasmine has expectAsync() which plays nicely with a promise-returning function, but it won't work with the Handler definition because of that void.
But you're right that it's trickier for people to understand when each handler should be used, so maybe it's better to just stick to the "do it yourself" solution instead of offering this in the types package.
Most simplified examples look contrived. I've been bitten by the lack of extra argument checking before in other code, generally when a package doesn't accurately document that the extra arguments will be passed. That to me is a strong signal that you should never lie about the actual arguments that get passed, and the people who aren't aware of callback are exactly the ones who should get the errors.
@coyoteecd this small npm package has a couple utility types that transform the @types/aws-lambda handler function types into the callback-only or async Promise-only signatures that we would actually implement. The've allowed me to have strongly typed methods with the interfaces from this project that can still be easily unit-tested.
Most helpful comment
No, because an optional argument in typescript means it's optional for the caller, because the implementing function must handle it not being present.
AWS is the caller, and they always provide it, so you, implementing the handler, don't need to handle it not being present.
If you don't want to do anything with an argument, you don't need to request it, e.g.