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

'context' keyword is stripped out from request body #897

Closed
2 tasks done
ayZagen opened this issue Oct 20, 2019 · 3 comments · Fixed by #910
Closed
2 tasks done

'context' keyword is stripped out from request body #897

ayZagen opened this issue Oct 20, 2019 · 3 comments · Fixed by #910
Labels
bug Something does not work as it should ✭ help wanted ✭
Milestone

Comments

@ayZagen
Copy link

ayZagen commented Oct 20, 2019

Describe the bug

  • Node.js version: 12.11.0
  • OS & version: ubuntu 18.04
  • got version: 10.0.0-alpha.2.2

context key is stripped from body.

Actual behavior

when we include context in our request body it is stripped out from outgoing request.
...

Code to reproduce

const got = require('got');
(async () => {
  let resp;
  try {

    resp = await got('http://google.com',{
      method : 'POST',
      json   : {
        context : { a: 'test'},
        a       : { b: 'asdasd'}
      }
    });
  } catch (e) {
    console.log(JSON.parse(e.options.body));
  }

  console.log(resp);
})();

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@ayZagen ayZagen changed the title 'context' keyword is stripped out from request 'context' keyword is stripped out from request body Oct 20, 2019
@szmarczak szmarczak added bug Something does not work as it should ✭ help wanted ✭ labels Oct 20, 2019
@ayZagen
Copy link
Author

ayZagen commented Oct 20, 2019

culprit is at merge.ts. Is it really necessary to check context in every source ?

sindresorhus added a commit that referenced this issue Nov 1, 2019
@sindresorhus
Copy link
Owner

I think the problem is that json, body, form, should not be passed through merge.ts at all. @szmarczak would know more.

I've added a failing test for this: cb042ac

@sindresorhus sindresorhus added this to the v10 milestone Nov 1, 2019
@szmarczak
Copy link
Collaborator

I'll investigate ASAP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should ✭ help wanted ✭
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants