fix: error on creating Cognito authorizers for V1 AWS APIs
Fixes the error discussed in https://github.com/sst/ion/issues/1144
Thanks
I've got this same issue in V1,
This is causing an issue when trying to create a Cognito Authoriser. When setting up a Cognito Authoriser we do not required a function, and in fact errors before this line if you try to add a token/request Function.
fn? will be rightly undefined in this case, and should not be force to be non null.
If I revert this change, and do the below, my authoriser is setup correctly and works.
api.addAuthorizer({
name: "my-authorizer",
userPools: [myUserPoolArn],
});
it does look like this issue could also be in V2 (http) of the apigateway function also.
I haven't tested it, but I don't think this is an issue in V2 API Gateway - in there, as the code currently stands, it looks like fn will only be undefined if lamb is undefined, and the fn! line is only hit if lamb is truthy. I think there's an argument to be made for avoiding the use of the non-null assertion operator as far as possible, to avoid bugs creeping in inadvertantly in future if the logic changes - but I'd rather keep this PR focused on fixing the specific bug we're hitting.
Maintainers - are you happy with this PR? Do I need to do anything further to get it merged?
I haven't tested it, but I don't think this is an issue in V2 API Gateway - in there, as the code currently stands, it looks like
fnwill only be undefined iflambis undefined, and thefn!line is only hit iflambis truthy. I think there's an argument to be made for avoiding the use of the non-null assertion operator as far as possible, to avoid bugs creeping in inadvertantly in future if the logic changes - but I'd rather keep this PR focused on fixing the specific bug we're hitting.Maintainers - are you happy with this PR? Do I need to do anything further to get it merged?
"argument to be made for avoiding the use of the non-null assertion operator as far as possible" 100% Agree.
Would appreciate this PR being released, as I've currently out of scoped auth for a specific API route because of this bug for a project I'm working on to carry on to meet project deadlines, we'll loop back once this is merged and released.
Would appreciate this PR being released, as I've currently out of scoped auth for a specific API route because of this bug for a project I'm working on to carry on to meet project deadlines, we'll loop back once this is merged and released.
Me too, as it's blocking me as well. I don't know how to proceed though - I've never contributed to this project before and am not sure of the process.
Will be in v3.2.29 - sorry about the delay!
thanks @fwang ! great news