Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom context for function handlers result in panic #377

Closed
d-tsuji opened this issue May 22, 2021 · 2 comments · Fixed by #475
Closed

Custom context for function handlers result in panic #377

d-tsuji opened this issue May 22, 2021 · 2 comments · Fixed by #475

Comments

@d-tsuji
Copy link
Contributor

d-tsuji commented May 22, 2021

Is your feature request related to a problem? Please describe.

In the AWS documentation, the handler for the function is shown to have the following signature

  • func ()
  • func () error
  • func (TIn) error
  • func () (TOut, error)
  • func (TIn) (TOut, error)
  • func (context.Context) error
  • func (context.Context, TIn) error
  • func (context.Context) (TOut, error)
  • func (context.Context, TIn) (TOut, error)

https://docs.aws.amazon.com/lambda/latest/dg/golang-handler.html

But it also says the following.

The handler may take between 0 and 2 arguments. If there are two arguments, the first argument must implement context.Context.


The first argument in the signature of the function that takes two arguments is clearly context.Context, but the panic occurs when customContext is taken as an argument.

For example, deploy and execute the following code to AWS Lambda.

package main

import (
	"context"
	"log"

	"github.com/aws/aws-lambda-go/lambda"
)

func main() {
	lambda.Start(Handle)
}

func Handle(ctx customContext) {
	log.Println("hello")
	return
}

// customContext satisfies "context.Context"
type customContext struct {
	context.Context
}

When this Lambda is executed, a panic occurs as shown below.

reflect: Call using *context.valueCtx as type main.customContext: string
[
	{
		"path": "github.com/aws/aws-lambda-go@v1.24.0/lambda/errors.go",
		"line": 39,
		"label": "lambdaPanicResponse"
	},
	{
		"path": "github.com/aws/aws-lambda-go@v1.24.0/lambda/function.go",
		"line": 36,
		"label": "(*Function).Invoke.func1"
	},
	{
		"path": "runtime/panic.go",
		"line": 965,
		"label": "gopanic"
	},
	{
		"path": "reflect/value.go",
		"line": 406,
		"label": "Value.call"
	},
	{
		"path": "reflect/value.go",
		"line": 337,
		"label": "Value.Call"
	},
	{
		"path": "github.com/aws/aws-lambda-go@v1.24.0/lambda/handler.go",
		"line": 124,
		"label": "NewHandler.func1"
	},
	{
		"path": "github.com/aws/aws-lambda-go@v1.24.0/lambda/handler.go",
		"line": 24,
		"label": "lambdaHandler.Invoke"
	},
	{
		"path": "github.com/aws/aws-lambda-go@v1.24.0/lambda/function.go",
		"line": 64,
		"label": "(*Function).Invoke"
	},
	{
		"path": "reflect/value.go",
		"line": 476,
		"label": "Value.call"
	},
	{
		"path": "reflect/value.go",
		"line": 337,
		"label": "Value.Call"
	},
	{
		"path": "net/rpc/server.go",
		"line": 377,
		"label": "(*service).call"
	},
	{
		"path": "runtime/asm_amd64.s",
		"line": 1371,
		"label": "goexit"
	}
]

Describe the solution you'd like

If we take context.Context as an argument, we need to make sure that the first argument of the function signature is context.Context explicitly.

contextType := reflect.TypeOf((*context.Context)(nil)).Elem()
argumentType := handler.In(0)
handlerTakesContext = argumentType.Implements(contextType)

The above implementation only checks if the argument satisfies context.Context, so if a custom context that satisfies context.Context is the first argument, it cannot be checked by the above validation.

Describe alternatives you've considered

I wasn't sure if it was correct to explicitly accept only context.Context as a signature, or if we will accept a custom context satisfying context.Context as a function signature.

Additional context

Nothing.

@shogo82148
Copy link
Contributor

The document and the implementation of the validator are wrong.

The document says:

https://docs.aws.amazon.com/lambda/latest/dg/golang-handler.html#golang-handler-structs
The handler may take between 0 and 2 arguments. If there are two arguments, the first argument must implement context.Context.

it should be:

The handler may take between 0 and 2 arguments. If there are two arguments, the first argument must be an interface and a subset of context.Context.

#365 will fix this.

@d-tsuji
Copy link
Contributor Author

d-tsuji commented May 22, 2021

@shogo82148

Thank you. I got the latest module as follows, and I can confirm that the panic doesn't happen in the custom context.

  • go.mod
module sample

go 1.16

require github.com/aws/aws-lambda-go v1.25.0

replace github.com/aws/aws-lambda-go => github.com/shogo82148/aws-lambda-go v1.19.2-0.20210522080913-8845e52c0275

shogo82148 added a commit to shogo82148/aws-lambda-go that referenced this issue Dec 21, 2022
bmoffatt pushed a commit that referenced this issue Dec 22, 2022
… result in a panic when constructing the context

* add test case for #377

* fix panicking

* use interface{} instead of any

* add a comment for argumentType.NumMethod() == 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants