typestack / routing-controllers

Create structured, declarative and beautifully organized class-based controllers with heavy decorators usage in Express / Koa using TypeScript and Routing Controllers Framework.
MIT License
4.39k stars 391 forks source link

fix: Cannot set headers after they are sent to the client #732

Closed leonardo2204 closed 3 years ago

leonardo2204 commented 3 years ago

Description

Somehow, using CORS in my code, some routes are throwing Cannot set headers after they are sent to the client.

Minimal code-snippet showcasing the problem

const staticControllers = [[`${parentDir}`, 'api', 'static', 'controllers', '**', `*.${ext}`].join(path.sep)]
        const middlewares = [[`${parentDir}`, 'api', 'middlewares', '**', `*.${ext}`].join(path.sep)]
        const interceptors = [[`${parentDir}`, 'api', 'interceptors', '**', `*.${ext}`].join(path.sep)]

        const appCopy: Express = useExpressServer(this.app, {
            routePrefix: '/static',
            controllers: staticControllers,
            authorizationChecker: staticTokenAuthChecker,
        })

        const controllers = [[`${parentDir}`, 'api', 'controllers', '**', `*.${ext}`].join(path.sep)]

        useExpressServer(appCopy, {
            routePrefix: config.express.routePrefix,
            cors: true, <--------- removing this line makes the problem go away
            authorizationChecker,
            currentUserChecker: currentUserChecker(this.userService),
            controllers,
            middlewares,
            interceptors,
        })
@R.Get('/today')
    async getTodayOrders(@QueryParam('token') token: string) {
        const verifiedToken = await webToken.decode(token.replace('Bearer ', ''))
        const { clientId } = verifiedToken

        const checklist = await this.orderService.getTodayOrdersByClientId(
            clientId,
        )

        if (
            checklist?.order?.length > 0
            && checklist.order[0].client_id !== clientId
        ) {
            throw new R.UnauthorizedError(
                'Client does not have permission to view this order',
            )
        }

        return checklist
    }

Expected behavior

CORS should not get in the way here

Actual behavior

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:363:5)
    at ServerResponse.setHeader (node:_http_outgoing:574:11)
    at applyHeaders (/Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:153:15)
    at applyHeaders (/Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:149:11)
    at applyHeaders (/Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:149:11)
    at cors (/Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:187:7)
    at /Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:224:17
    at originCallback (/Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:214:15)
    at /Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:219:13
    at optionsCallback (/Users/leonardo2204/floki/backend/node_modules/cors/lib/index.js:199:9)

I'm really trying to figure out why (and when) cors is getting called here.

Any help is much appreciated!

sittingbool commented 3 years ago

This happened to me only when I did not remove the default express router und then two router-"middlewares" have been excuted for the same route.

leonardo2204 commented 3 years ago

Yeah, it seems to be the problem. In my case, changing the order solved the problem:

        const appCopy = useExpressServer(this.app, {
            routePrefix: config.express.routePrefix,
            authorizationChecker: authorizationChecker(this.authFactory),
            currentUserChecker: currentUserChecker(this.authFactory),
            controllers,
            middlewares,
            interceptors,
            cors: true,
        })

         useExpressServer(appCopy, {
            routePrefix: '/static',
            controllers: staticControllers,
            authorizationChecker: staticTokenAuthChecker,
        })

I did not test, but I think that only adding cors in the first useExpressServer should solve it. Anyway, it really feels like a bug, the framework should ignore if the user calls (by mistake) cors (or any other config boolean) twice.

Thanks for the answers!

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.