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

fix: event.resource in catch-all route gets + changed to * #1524

Merged
merged 4 commits into from
Aug 5, 2022

Conversation

Ankcorn
Copy link
Contributor

@Ankcorn Ankcorn commented Aug 3, 2022

Description

Replaces the * added by the generateHapiPath util for rest apis as this does not match the events from AWS

Motivation and Context

I ran into this bug event.resource in catch-all route gets + changed to * whilst testing out some changes to TRPC and their lambda adaptor.

How Has This Been Tested?

I added a new e2e test to cover the described behavior and it asserts that the resource contains the + character as expected

Screenshots (if appropriate):

N/A

fixes: #1431

@@ -146,7 +146,11 @@ export default class LambdaProxyIntegrationEvent {
const httpMethod = method.toUpperCase()
const requestTime = formatToClfTime(received)
const requestTimeEpoch = received
const resource = this.#routeKey || route.path.replace(`/${this.#stage}`, '')
// const resource = this.#routeKey || route.path.replace(`/${this.#stage}`, '')
// NOTE Removed * added by generateHapiPath so api gateway event is accurategenerateHapiPath
Copy link
Collaborator

Choose a reason for hiding this comment

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

// NOTE Removed Replaced...?
what is accurategenerateHapiPath?

tests/endToEnd/starRoutes/starRoutes.test.js Outdated Show resolved Hide resolved
@dnalborczyk
Copy link
Collaborator

dnalborczyk commented Aug 4, 2022

thank you @Ankcorn ! added some comments, otherwise looks good.

do you know if this issue also applies to httpApi? just tried, I guess it doesn't.

@dnalborczyk dnalborczyk changed the title Fix: #1431 event.resource in catch-all route gets + changed to * fix: event.resource in catch-all route gets + changed to * Aug 4, 2022
@Ankcorn
Copy link
Contributor Author

Ankcorn commented Aug 4, 2022

woops a few typo's here, serves me right for making PR's at midnight. I have fixed those up, thanks for checking the http api :)

@Ankcorn Ankcorn requested a review from dnalborczyk August 4, 2022 21:39
@dnalborczyk
Copy link
Collaborator

woops a few typo's here, serves me right for making PR's at midnight.

happens to me fairly often as well 😆

@dnalborczyk dnalborczyk merged commit 0494fdb into dherault:master Aug 5, 2022
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.

event.resource in catch-all route gets + changed to *
2 participants