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

Can not set response headers when send a json body in a options request #5626

Open
yhojann-cl opened this issue Apr 28, 2024 · 6 comments
Open

Comments

@yhojann-cl
Copy link

yhojann-cl commented Apr 28, 2024

By example:

router.options('/', (req, res, next) => {
    res
        .status(200)
        .setHeader('Access-Control-Allow-Methods', allowFor('/')) // Without this line works fine.
        .json({
            schema: schemafor('/')
        });
});

Says:

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
at ServerResponse.setHeader (node:_http_outgoing:652:11)
at ServerResponse.header (/.../node_modules/express/lib/response.js:794:10)
at sendOptionsResponse (/.../node_modules/express/lib/router/index.js:654:9)
at Immediate. (/.../node_modules/express/lib/router/index.js:167:7)
at Immediate.proxy (/.../node_modules/express/lib/router/index.js:671:8)
at process.processImmediate (node:internal/timers:480:21)

The RFC allows the return of http content for options requests. I use it to return crud data schema settings for the frontend and allowed methods for show or hide pages in the dashboard.

@yhojann-cl yhojann-cl added the bug label Apr 28, 2024
@yhojann-cl yhojann-cl changed the title Can not set response headers when send a json body Can not set response headers when send a json body in a options request Apr 28, 2024
@wesleytodd
Copy link
Member

Yeah I believe you are correct, but it would be good if you could get the specific section of the spec for this so we can align on the expectations. I did a cursory search and found this which I think is the right place. It has been a long time since I have looked at these and this doc is new to me.

Additionally I would say I am surprised this is broken. I don't remember any specific options handling here and setHeader is not an express api but comes directly from node. It might be worth checking with a reduced test case what the behavior is without express.

@danizavtz
Copy link

hello @yhojann-cl , can you detail what functions allowFor and schemafor are? What is the source code of these functions?

I'm guessing that maybe not using the function json works, because when you use this function it finishes the response cicle and send a response. As explicit we can see in this code

So maybe, you can try to use the function write to populate a response as you want, and after finish you can send the response explicit calling function end().

As an example:

//not tested code
router.options('/', (req, res, next) => {
    res
        .status(200)
        .setHeader('Access-Control-Allow-Methods', allowFor('/')) // Without this line works fine.
        .setHeader('Content-Type', 'application/json')
        .write({
            schema: schemafor('/')
        })
       //write more stuff, case needed, then finish response with end()...
       res.end();
       
});

@Codexnever
Copy link

router.options('/', (req, res, next) => {
    // Set the status code and headers
    res
        .status(200) // Set status code to 200 OK
        .setHeader('Access-Control-Allow-Methods', allowFor('/')) // Set allowed methods header
        .setHeader('Content-Type', 'application/json'); // Set content type header to JSON

    // Manually construct the JSON response
    res.write(JSON.stringify({
        schema: schemafor('/') // Populate JSON response with schema data
    }));

    // End the response
    res.end(); // Complete response
});

@AbdLaswi
Copy link

@yhojann-cl I modified your code to be the following ( cuz I don't the values of schemafor and allowFor ):
router.options('/', (req, res, next) => { res .status(200) .setHeader('Access-Control-Allow-Methods', '/') .json({ schema: "schemafor('/')" }); });

and it worked fine, without adding "res.end() or res.write()"

@coltonehrman
Copy link

@lochansaroy02
Copy link

lochansaroy02 commented Aug 24, 2024

    res.setHeader('Access-Control-Allow-Methods', allowFor('/'));
    
    res.status(200).json({
        schema: schemafor('/')
    });
});

Setting Headers Before Sending the Response: The header is set before calling .status(200) and .json(), ensuring that the headers are all properly configured before the response is sent to the client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants