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

aws-lambda should catch exceptions with 'httpCode' information, and correctly emit http errors (if no error middleware is used). #28

Open
universalhandle opened this issue Mar 2, 2021 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@universalhandle
Copy link
Contributor

Hello.

I saw curveball/core#92, but I must be missing something. I've got code like this:

// handler.ts
import { Application, invokeMiddlewares } from '@curveball/core';
import handler from '@curveball/aws-lambda';
import { fooRoutes } from './components/foo/routes';

const app = new Application();
app.use(ctx => invokeMiddlewares(ctx, fooRoutes));

export const router = handler(app);

// components/foo/routes.ts
import router from '@curveball/router';
import { fooController } from './controller'; // just has a "get" method

export const fooRoutes = [
  router('/:id/foo', fooController),
];

The relevant bit of serverless.yml looks like so:

functions:
  my-function:
    handler: src/handler.router
    events:
      - http:
          # sends everything through to the router
          path: /{proxy+}
          method: any

When I visit localhost:3000/12345/foo, I get the expected result.

When I visit any other path (note: I've defined none), I get a 200 and an empty page. I would expect a 404 or a 405. Is handling of nonexistent routes not automagic? Did I miss a step?

If it's not handled automatically, is there a recommend best practice? (I didn't try too long, but a wildcard route didn't seem to do the trick either.)

Thanks!

@evert evert transferred this issue from curveball/router Mar 2, 2021
@evert evert added the bug Something isn't working label Mar 2, 2021
@evert
Copy link
Member

evert commented Mar 2, 2021

When @curveball/core executes all middlewares, and reaches all the way to the end it runs a 'NotFound' middleware.
tt
This middleware throws an exception. The 'normal curveball http server' path catches any exception, and emits a plain text error + http status code.

The @curveball/aws-lambda path does not catch this exception and do anything with this. Normally this would simply cause the lambda execution to fail, but I suspect that the serverless framework catches it, and return the 200 maybe? It is weird that it would do that!

A quick solution to this is to add the @curveball/problem middleware. This middleware catches errors and turns them into nicer looking application/problem+json errors.

I do think that @curveball/aws-lambda should behave as the regular server though, and emit errors that have a httpStatus property, but it also makes sense to me that other uncaught exceptions should bubble up to AWS Lambda itself as a critical failure, so it will show up as such in monitoring utilities.

tl;dr:

  1. There is a bug here.
  2. A quick workaround is to use @curveball/problem.
  3. The fix for this should not catch non-http errors, so if the parent framework eats up exceptions and turns them into 200 OK, that's still pretty weird to me!

@evert evert self-assigned this Mar 2, 2021
@evert evert changed the title Best practice for nonexistent routes, methods, etc? aws-lambda should catch exceptions with 'httpCode' information, and correctly emit http errors (if no error middleware is used). Mar 2, 2021
@universalhandle
Copy link
Contributor Author

Hey, thanks for your super speedy response!

You got me thinking and I tried a few more things. Not sure if any of this is helpful, but I doubt it can hurt!

Note that the example I gave above has serverless passing everything through via the path: /{proxy+} configuration. For everything that follows, I have set http events explicitly per below:

functions:
  my-function:
    handler: src/handler.router
    events:
      - http:
          path: /{id}/foo
          method: any
      - http:
          path: /bar
          method: any

For a path /unknown/to/both/serverless/and/curveball I get a 404 and a message Serverless-offline: route not found.

Suppose I forget to add a route to my curveball config for path /bar. In this case, Chrome gives me a 200 and a blank page. Firefox gives me a 200 and asks me to download a zero-length file:

image

The problem middleware doesn't seem to do the trick for me. I've updated my code as follows, and I see no change in behavior:

// handler.ts
import { Application, invokeMiddlewares } from '@curveball/core';
import handler from '@curveball/aws-lambda';
import problemMiddleware from '@curveball/problem';
import { fooRoutes } from './components/foo/routes';

const app = new Application();
const middlewares = [ problemMiddleware(), ... fooRoutes ];
app.use(ctx => invokeMiddlewares(ctx, middlewares));

export const router = handler(app);

If you're not able to reproduce, I could set up a code sandbox somewhere. If you have a preferred tool for that, let me know.

Thanks again for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants