CodeGenieApp / CodeGenieApp/serverless-express

Routing to root path does not work correctly when using nested API resources

Aperta
#377 0 commenti 6 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
JavaScript
Stelle
5.3k
Fork
676
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

First off, thanks for this great project!

I'm reporting an issue here I ran into when using API Gateway v1 with a proxy resource.

### Repro

I have defined a RestAPI resource triggered on a **nested** root path and subpaths under a `/v1/` path namespace. The SAM template looks like this:

```
Events:
ProxyApiRoot:
Type: Api
Properties:
RestApiId: !Ref ExpressApi
Path: /v1/
Method: ANY
ProxyApiGreedy:
Type: Api
Properties:
RestApiId: !Ref ExpressApi
Path: /v1/{proxy+}
Method: ANY
```

I also have a BasePathMapping defined nesting one level deeper, so the final root path for my app is `/routes/v1/`.

My express app has a handler defined for the root path:

```
router.get('/', (req, res) => { ... }
```

However a request to `/routes/v1/` returns a generic 404 from express. The request is routing correctly through API Gateway to my app, but once it gets to express it can't find a matching handler for the `/` path.

### Debugging

I added a handler for the catch-all route `*` and printed out the currentInvoke to try to track down the problem. I think I narrowed it down to this line:

https://github.com/vendia/serverless-express/blob/4ba9799a6573ea4227939001c3e49e07b75a35ea/src/event-sources/utils.js#L6-L7

The issue is that my request to `/routes/v1/` is being handled by ProxyApiRoot, not ProxyApiGreedy. Since ProxyApiRoot doesn't populate a value for event.pathParameters.proxy, the fallback event.path is used. But unlike pathParameters.proxy, event.path includes the _full path_ including the namespaces. So for a request to the root of my app, serverless-express is interpreting the path as `event.path == '/routes/v1'`, not `event.path == '/'`.

### Solution?

There may need to be more smarts added to `getPathWithQueryStringParams` for stripping out prefixes from the root path.

For example from printing out the currentInvoke I found:
* The base path part is available at `event.requestContext.customDomain.basePathMatched == 'routes'`
* The `v1` part is available at `event.resource = '/v1'`.

Perhaps these can be used to remove `/routes/v1` from the root path when event.pathParameters.proxy is not populated.

### Workaround

In the short term I found a workaround by avoiding root paths altogether. My SAM template now looks like:

```
ProxyApiGreedy:
Type: Api
Properties:
RestApiId: !Ref ExpressApi
Path: /{proxy+}
Method: ANY
```

and I mount all my express routes at v1 explicitly using `app.use('/v1', router)`.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Start in src/event-sources/utils.js at getPathWithQueryStringParams, then reproduce the nested /v1/ and /v1/{proxy+} SAM API Gateway routes with the /routes/v1/ base path mapping. Trace the event fields used for the root request, especially event.path, event.resource, event.requestContext.customDomain.basePathMatched, and event.pathParameters.proxy. Done means the Express root handler receives / rather than the namespaced path.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
aws, express, javascript, node.js
Ambito
api, backend
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.