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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

.end overwrites res.body with the last character #26

Closed
ErisDS opened this issue Jan 25, 2022 · 1 comment 路 Fixed by #27
Closed

.end overwrites res.body with the last character #26

ErisDS opened this issue Jan 25, 2022 · 1 comment 路 Fixed by #27
Labels
bug Something isn't working released

Comments

@ErisDS
Copy link
Collaborator

ErisDS commented Jan 25, 2022

I've been experimenting with reqresnext as a way to write performant tests against express.

Today I ran into a weird issue where I suddenly started only getting the very last character of my content 馃

It was happening every time I wired up express-session to my express app. It seems that once you add session handling both res.write and res.end are called internally.

The code here in reqresnext overwrites body on res.end rather than concatenating and res.end is only getting called with the final character.

My minimal repo case looks something like this:

// app.js
const express = require('express');
const session = require('express-session');

const app = express();

app.use(express.json());

app.use(session({
    secret: 'verysecretstring',
    name: 'testauth'
}));

app.get('/', (req, res) => {
    return res.send('Hello World!');
});
// reqres.js

const {Request, Response} = require('reqresnext');

const app = require('./app.js');

const req = new Request(Object.assign({}, {url: '/', method: 'GET'}, {app}));
const res = new Response(Object.assign({}, {}, {app, req}));

res.on('finish', () => {
    const text = res.body.toString('utf8');

    console.log('TEXT', text);
});

app(req, res);

You'll get "TEXT !" as the output.

If you comment out the lines


app.use(session({
    secret: 'verysecretstring',
    name: 'testauth'
}));

You'll get "TEXT Hello World!" as expected.

I'm able to fix it by changing adding this to reqres.js

    if (chunk) {
        res.write(chunk, encoding);
    }
    res.emit('finish');
    return this;
};
ErisDS added a commit to ErisDS/reqresnext that referenced this issue Jan 25, 2022
fixes: antongolub#26

- added a test to demonstrate how write and end are used together that should fail with the current code
- use the already bound write method to ensure that body is concatenated correctly
- this seemed like the quickest way to do this, as callback isn't used
@antongolub antongolub added the bug Something isn't working label Jan 25, 2022
antongolub pushed a commit that referenced this issue Jan 26, 2022
fixes: #26

- added a test to demonstrate how write and end are used together that should fail with the current code
- use the already bound write method to ensure that body is concatenated correctly
- this seemed like the quickest way to do this, as callback isn't used
@antongolub
Copy link
Owner

馃帀 This issue has been resolved in version 1.6.5 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

ErisDS added a commit to ErisDS/framework that referenced this issue Jan 26, 2022
- the main change here is to add express-session and a login and authenticated endpoint so I can start figuring out how to get the agent to maintain cookies
- however I found a really weird issue where as soon as you add express-session then express starts acting differently & breaks reqresnext
- at this point both res.write and res.end get called, where res.end only gets the last char
- because reqresnext's end method doesn't concatenate with write, you only get the last char returned
- therefore I've had to add a workaround in lieu of antongolub/reqresnext#26 being looked at
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants