curveball / core

The Curveball framework is a TypeScript framework for node.js with support for modern HTTP features.
https://curveballjs.org/
MIT License
526 stars 7 forks source link

Header values are output twice #175

Closed universalhandle closed 3 years ago

universalhandle commented 3 years ago

Hi, I'm not sure which repo to raise this issue in, so I went for core.

I'm using problem and getting response headers with doubled values. Both Postman and my browser console shows headers like:

cache-control: no-cache
Connection: keep-alive
content-length: 75
content-type: application/problem+json,application/problem+json
Date: Mon, 15 Mar 2021 22:38:33 GMT
Keep-Alive: timeout=5
server: curveball/0.16.2,curveball/0.16.2
www-authenticate: Basic; realm="foo",Basic; realm="foo"

... where I'd expect to see...

cache-control: no-cache
Connection: keep-alive
content-length: 75
content-type: application/problem+json
Date: Mon, 15 Mar 2021 22:38:33 GMT
Keep-Alive: timeout=5
server: curveball/0.16.2
www-authenticate: Basic; realm="foo"

I've got code like this:

// 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);

// components/foo/routes.ts
import router from '@curveball/router';
import { auth } from '../../middleware/auth'; // throws new Unauthorized()
import { fooController } from './controller'; // just has a "get" method

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

I did a little debugging and determined:

Have you seen anything like this before? Happy to help fix it (if it is not in fact a bug in my own code), but could use a little hint as to where the problem may lie.

Thank you!

evert commented 3 years ago

Hi!

I'm a little confused about this too. Definitely haven't seen this. The obvious idea would be that problem is somehow loaded twice... I don't suppose there's some other place in your code that may have happened?

evert commented 3 years ago

Also, this is probably not the issue, but any reason you're not adding the middlewares this way?

const app = new Application();
app.use(problemMiddleware());
app.use(...fooRoutes);
universalhandle commented 3 years ago

The obvious idea would be that problem is somehow loaded twice... I don't suppose there's some other place in your code that may have happened?

Nope. Code base is still small but I grepped it anyway to be sure. It appears only in handler.ts.

Also, this is probably not the issue, but any reason you're not adding the middlewares this way?

No reason. I must have somehow missed that use can handle multiple middlewares. Unfortunately, this change does not alter the output.

For what it's worth, the header value doubling only manifests in the browser/postman. When I trigger it, my server console looks like this:

Serverless: GET /my-route (λ: my-lambda)
Unauthorized [Error]: Authentication is required
  // stack trace
  type: null,
  httpStatus: 401,
  title: 'Unauthorized',
  detail: 'Authentication is required',
  instance: null,
  wwwAuthenticate: 'Basic; realm="foo"'
}

I don't think the stack trace is the place to debug this, though I can share it if you think it'll help. I'm guessing the place to look is in problem or core -- wherever the error is caught, not where it's thrown. Sound right? Can you point me in the right direction?

evert commented 3 years ago

Yes!

This is where errors are caught. It's just 1 file:

https://github.com/curveball/problem/blob/master/src/index.ts#L25

It's also very strange that headers only double in postman.... very suspicious!

universalhandle commented 3 years ago

I found that this issue only occurs when running the app in serverless offline. It's not reproducible in AWS proper. I haven't dug around further to get to the root of it, but I thought I'd close this issue since the behavior seems unlikely to be affected by changes to curveball.

Also, for posterity: the repetition is not limited to error responses; content-type: application/xml,application/xml with a 200 status can also happen.