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

AmazonLambdaRecorder Handler Discovery Erroneously Considers Decorators #34824

Closed
gilday opened this issue Jul 18, 2023 · 3 comments · Fixed by #44155
Closed

AmazonLambdaRecorder Handler Discovery Erroneously Considers Decorators #34824

gilday opened this issue Jul 18, 2023 · 3 comments · Fixed by #44155
Labels
area/amazon-lambda kind/bug Something isn't working
Milestone

Comments

@gilday
Copy link

gilday commented Jul 18, 2023

Describe the bug

In the absence of any configuration, the io.quarkus.amazon.lambda.runtime.AmazonLambdaRecorder automatically discovers the application's single RequestHandler or RequestStreamHandler class. If multiple candidate types are discovered this way, then the AmazonLambdaRecorder errors:

java.lang.RuntimeException: Multiple handler classes, either specify the quarkus.lambda.handler property, or make sure there is only a single com.amazonaws.services.lambda.runtime.RequestHandler or, com.amazonaws.services.lambda.runtime.RequestStreamHandler implementation in the deployment

Presently, this type discovery logic considers RequestHandler and RequestStreamHandler types that are CDI decorators (i.e. annotated with jakarta.decorator.Decorator). This doesn't seem right, because the decorator type itself can never be the request handler, absent the handler it decorates.

This behavior is a problem for application developers that want to seamlessly introduce new decorators for their Lambda function handlers, because the introduction of the decorator unexpectedly requires the application develop to name all lambda function handler type beans and introduce the quarkus.lambda.handler configuration property.

Expected behavior

AmazonLambdaRecorder ignores CDI decorator types in its RequestHandler and RequestStreamHandler type discovery. Therefore, it selects the first not-decorator RequestHandler or RequestStreamHandler bean, and, of course, that is decorated with any applicable jakarta.decorator.Decorator beans that may exist.

Actual behavior

Quarkus reports a conflict between the RequestHandler or RequestStreamHandler and the decorator, and this conflict must be resolved using quarkus.lambda.handler to specify the not-decorator type.

How to Reproduce?

lambda-request-handler-decorator.zip

Output of uname -a or ver

Darwin pixee-mbp-gilday.localdomain 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000 arm64

Output of java -version

openjdk version "17.0.7" 2023-04-18 OpenJDK Runtime Environment Temurin-17.0.7+7 (build 17.0.7+7) OpenJDK 64-Bit Server VM Temurin-17.0.7+7 (build 17.0.7+7, mixed mode)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.2.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.8 (4c87b05d9aedce574290d1acc98575ed5eb6cd39) Maven home: /Users/jgilday/.m2/wrapper/dists/apache-maven-3.8.8-bin/67c30f74/apache-maven-3.8.8 Java version: 17.0.7, vendor: Eclipse Adoptium, runtime: /Library/Java/JavaVirtualMachines/temurin-17.jdk/Contents/Home Default locale: en_US, platform encoding: UTF-8 OS name: "mac os x", version: "13.4.1", arch: "aarch64", family: "mac"

Additional information

No response

@gilday gilday added the kind/bug Something isn't working label Jul 18, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 18, 2023

/cc @matejvasek (amazon-lambda), @patriot1burke (amazon-lambda)

@gilday
Copy link
Author

gilday commented Jul 20, 2023

This is also an issue at build time for Lambda functions that use the AWS Lambda HTTP extension: decorator beans on the classpath that would otherwise do nothing cause build failures.

Caused by: io.quarkus.builder.BuildException: Build failure: Multiple handler classes.  You have a custom handler class and the AWS Lambda HTTP extension.  Please remove one of them from your deployment.
        at io.quarkus.amazon.lambda.deployment.AmazonLambdaProcessor.discover(AmazonLambdaProcessor.java:88)

@gsmet
Copy link
Member

gsmet commented Oct 29, 2024

It took a while, sorry, but I created #44155 to hopefully take care of it.

Thanks for the awesome reproducer!

@gsmet gsmet closed this as completed in cf0948c Oct 30, 2024
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Oct 30, 2024
@gsmet gsmet modified the milestones: 3.17 - main, 3.16.2 Nov 5, 2024
gsmet added a commit to gsmet/quarkus that referenced this issue Nov 5, 2024
We need to ignore decorators and abstract classes when selecting the
request handlers.

Fixes quarkusio#34824

(cherry picked from commit cf0948c)
benkard pushed a commit to benkard/quarkus-googlecloud-jsonlogging that referenced this issue Nov 13, 2024
…us-googlecloud-jsonlogging!24)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io.quarkus:quarkus-extension-processor](https://github.com/quarkusio/quarkus) |  | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-extension-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-bom](https://github.com/quarkusio/quarkus) | import | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.16.1` -> `3.16.3` |

---

### Release Notes

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.16.3`](quarkusio/quarkus@3.16.2...3.16.3)

[Compare Source](quarkusio/quarkus@3.16.2...3.16.3)

### [`v3.16.2`](https://github.com/quarkusio/quarkus/releases/tag/3.16.2)

[Compare Source](quarkusio/quarkus@3.16.1...3.16.2)

##### Complete changelog

-   [#&#8203;34824](quarkusio/quarkus#34824) - AmazonLambdaRecorder Handler Discovery Erroneously Considers Decorators
-   [#&#8203;38086](quarkusio/quarkus#38086) - Documentation about `RecordCodecProvider` in MongoDB with Panache
-   [#&#8203;42149](quarkusio/quarkus#42149) - Upgrade Postgres 16
-   [#&#8203;44039](quarkusio/quarkus#44039) - WebSockets Next: create a new event loop context for each client
-   [#&#8203;44132](quarkusio/quarkus#44132) - Update getting-started-reactive.adoc
-   [#&#8203;44149](quarkusio/quarkus#44149) - Fix Config Error Screen
-   [#&#8203;44152](quarkusio/quarkus#44152) - Do not throw NPE in AfterAll interceptor if application didn't start
-   [#&#8203;44155](quarkusio/quarkus#44155) - Amazon Lambda - Support decorators
-   [#&#8203;44156](quarkusio/quarkus#44156) - Make OidcRequestContextProperties modifiable
-   [#&#8203;44178](quarkusio/quarkus#44178) - Bump com.fasterxml.jackson:jackson-bom from 2.18.0 to 2.18.1
-   [#&#8203;44183](quarkusio/quarkus#44183) - Properly apply the update recipes in version order
-   [#&#8203;44184](quarkusio/quarkus#44184) - Add Support for Trusted Proxy Detection on Forwarded Requests
-   [#&#8203;44185](quarkusio/quarkus#44185) - Repeating `@PermissionsAllowed` annotations totally disable method authentication
-   [#&#8203;44189](quarkusio/quarkus#44189) - Small improvements to the Deploying to Google Cloud guide
-   [#&#8203;44190](quarkusio/quarkus#44190) - ResteasyReactiveProcessor#setupEndpoints reports duplicate endpoints when a rest client matches a resource
-   [#&#8203;44191](quarkusio/quarkus#44191) - Update PostgreSQL image to 17
-   [#&#8203;44201](quarkusio/quarkus#44201) - Broken link
-   [#&#8203;44202](quarkusio/quarkus#44202) - Broken link?
-   [#&#8203;44203](quarkusio/quarkus#44203) - Make OidcRequestContextProperties modifiable
-   [#&#8203;44204](quarkusio/quarkus#44204) - Fix broken doc links
-   [#&#8203;44207](quarkusio/quarkus#44207) - Bump bouncycastle.version from 1.78.1 to 1.79
-   [#&#8203;44209](quarkusio/quarkus#44209) - Bump io.quarkus.develocity:quarkus-project-develocity-extension from 1.1.6 to 1.1.7
-   [#&#8203;44221](quarkusio/quarkus#44221) - Add extension description for websockets next
-   [#&#8203;44227](quarkusio/quarkus#44227) - Add stork-configuration-generator as an annotationProcessorPath
-   [#&#8203;44229](quarkusio/quarkus#44229) - ContainerResponseFilter with `@Priority(Integer.MIN_VALUE)` will be actually invoked with max priority
-   [#&#8203;44232](quarkusio/quarkus#44232) - Quartz: use a more reasonable default for quarkus.quartz.thread-count
-   [#&#8203;44235](quarkusio/quarkus#44235) - 3.16: `@WithTestResource` starts all test resources (regression)
-   [#&#8203;44237](quarkusio/quarkus#44237) - Properly implement priority of ContainerResponseFilter
-   [#&#8203;44238](quarkusio/quarkus#44238) - Refactor SecurityTransformerUtils to consider repeated annotations
-   [#&#8203;44239](quarkusio/quarkus#44239) - Replace oidc auth facebook screenshots with generic ones
-   [#&#8203;44244](quarkusio/quarkus#44244) - Bump `quarkiverse-parent` to 18
-   [#&#8203;44245](quarkusio/quarkus#44245) - Delete disabled job
-   [#&#8203;44248](quarkusio/quarkus#44248) - Ignore client interfaces when detecting duplicate endpoints
-   [#&#8203;44263](quarkusio/quarkus#44263) - Quarkus Dev UI - Clicking on gRPC - Services - service implementation class  Uncaught exception received by Vert.x
-   [#&#8203;44277](quarkusio/quarkus#44277) - Dev UI Open in IDE make sure lineNumber is in quotes
-   [#&#8203;44279](quarkusio/quarkus#44279) - Limit `MATCHING_RESOURCES` TestResources to the test that declares them
-   [#&#8203;44281](quarkusio/quarkus#44281) - Included pages within a fragment ignores rendered=false property.
-   [#&#8203;44298](quarkusio/quarkus#44298) - Qute: fix rendered=false if a fragment includes nested fragment
-   [#&#8203;44300](quarkusio/quarkus#44300) - When testing request payload is populated with string "null" if enable-reflection-free-serializers enabled
-   [#&#8203;44309](quarkusio/quarkus#44309) - Avoid deserializing null nodes in reflection free Jackson serialization
-   [#&#8203;44316](quarkusio/quarkus#44316) - Duplicated field serialization using the generated reflection free Jackson serializers
-   [#&#8203;44317](quarkusio/quarkus#44317) - Avoid duplicated field serialization in reflection free Jackson serializers
-   [#&#8203;44321](quarkusio/quarkus#44321) - Use Java 21 by default in the Deploying to Google Cloud guide
-   [#&#8203;44322](quarkusio/quarkus#44322) - Explain in MongoDB docs that records are supported
-   [#&#8203;44324](quarkusio/quarkus#44324) - Take config annotation when trying to match test resources

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
bschuhmann pushed a commit to bschuhmann/quarkus that referenced this issue Nov 16, 2024
We need to ignore decorators and abstract classes when selecting the
request handlers.

Fixes quarkusio#34824
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/amazon-lambda kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants