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

feat(adaptor): Enhance AWS Lambda Event Handling and Interface #1710

Merged
merged 13 commits into from
Nov 27, 2023

Conversation

watany-dev
Copy link
Contributor

@watany-dev watany-dev commented Nov 16, 2023

fixed #1620

To provide full access to the Event, which is the input for AWS Lambda, the following modifications have been made:

1. Review of the Event

  • Confirmed that having API Gateway versions 1.0 and 2.0 is sufficient after reviewing the specifications. The official documentation states that the Event for the traditional Lambda URL is same to version 2.0.

  • Added missing properties in the Event Interface.

  • Renamed the externally exported LambdaFunctionUrlRequestContext to ApiGatewayRequestContextV2 for name alignment with Event.

    • This is a breaking change, so concurrently using the old name's copied Type for a certain period might be beneficial.

2. Access to the Event

  • Defined as export type LambdaEvent = APIGatewayProxyEvent | APIGatewayProxyEventV2 and exported in the index.
    • Allows users to specify using Bindings.

3. Discontinuation of requestContext (Not Yet Implemented)

  • Currently obtained as follows, which is cumbersome as it requires users to choose between v1 and v2 for the request context.
import { Hono } from 'hono'
import type { ApiGatewayRequestContext } from 'hono/aws-lambda'
import { handle } from 'hono/aws-lambda'

type Bindings = {
  requestContext: ApiGatewayRequestContext 
}

const app = new Hono<{ Bindings: Bindings }>()

app.get('/custom-context/', (c) => {
  const lambdaContext = c.env.requestContext
  return c.json(lambdaContext)
})

export const handler = handle(app)

With access to the event now available, this can be rewritten simply. Also, the internal unnecessary logic getRequestContext(event) can be removed.

import { Hono } from 'hono'
// import type { ApiGatewayRequestContext } from 'hono/aws-lambda'
import type { LambdaEvent } from 'hono/aws-lambda'
import { handle } from 'hono/aws-lambda'

type Bindings = {
  event: LambdaEvent 
//   requestContext: ApiGatewayRequestContext 
}

const app = new Hono<{ Bindings: Bindings }>()

app.get('/custom-context/', (c) => {
//   const lambdaContext = c.env.requestContext
  const lambdaContext = c.env.event.requestContext
  return c.json(lambdaContext)
})

export const handler = handle(app)

If the removal of requestContext is approved, a comment like "Remove in Hono V4" can be added.

Author should do the followings, if applicable

  • Add tests
  • Run tests
  • yarn denoify to generate files for Deno

@yusukebe
Copy link
Member

Hi @watany-dev

I'll review this later. It will be merged into a "next" branch because this will be new feature.

@yusukebe
Copy link
Member

yusukebe commented Nov 21, 2023

Hi @watany-dev !

Could you fix the conflict? After fixing it, I'll merge this into a "next" branch for the v3.11.0.

@yusukebe yusukebe changed the base branch from main to next November 21, 2023 09:05
@watany-dev
Copy link
Contributor Author

@yusukebe
Thanks. I fixed it.

* Invalid HTTP header error is hidden

This error is captured and never shown which may by misleading. This PR suggest to throw this error directly to the reponse.

* fixed the linter error

* add a test

* denoify

---------

Co-authored-by: Yusuke Wada <yusuke@kamawada.com>
Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@watany-dev

Sorry, one thing.

This is a breaking change, so concurrently using the old name's copied Type for a certain period might be beneficial.

I think we should do that. Let's make LambdaFunctionUrlRequestContext available and mark it as deprecated.

@yusukebe yusukebe self-requested a review November 21, 2023 11:43
@watany-dev
Copy link
Contributor Author

watany-dev commented Nov 22, 2023

@yusukebe
Yes, I was able to indicate the deprecation to the user.

@watany-dev
Copy link
Contributor Author

After fully supporting the API Gateway V1 event, we found that the traditional support for ALB was no longer possible. Therefore, we have defined a new schema.

@yusukebe
Copy link
Member

Hi @watany-dev

Sorry for the late reply. I am sick in bed right now. Will review when I'm better. Thanks.

Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yusukebe
Copy link
Member

@watany-dev

I'm back. This great! Let's go with it.

@yusukebe yusukebe merged commit bea87a0 into honojs:next Nov 27, 2023
11 checks passed
@yusukebe
Copy link
Member

yusukebe commented Dec 3, 2023

Hi @watany-dev

I'll release the new version v3.11.0 soon. If there are any descriptions on the website that need updating, could you please create a PR for them?

@watany-dev watany-dev deleted the refactor-lambda-adaptor branch December 4, 2023 03:48
@watany-dev
Copy link
Contributor Author

@yusukebe
OK! I wrote this Update
honojs/website#172

@yusukebe
Copy link
Member

yusukebe commented Dec 4, 2023

@watany-dev

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS Lambda Adapter only keeps requestContext from API GW event and loses the rest
3 participants