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

In .send(obj), obj's toJSON method is not called #1464

Closed
hasahmed opened this issue Feb 26, 2019 · 5 comments · Fixed by DeviaVir/zenbot#2014 · May be fixed by ajesse11x/core#1 or zhiliv/admin.kreditor.store#1
Closed

In .send(obj), obj's toJSON method is not called #1464

hasahmed opened this issue Feb 26, 2019 · 5 comments · Fixed by DeviaVir/zenbot#2014 · May be fixed by ajesse11x/core#1 or zhiliv/admin.kreditor.store#1

Comments

@hasahmed
Copy link

I have an object that has a circular property. Because of that I need a toJSON method that handles that particular field appropriately. Super agents send method doesn't call my objects toJSON method and therefore the call to JSON.strigify inside of superagent its self throws an error.

I attempted to fix this myself by calling data.toJSON inside of request-base.js:570 when the properties are copied into the requests data, but this lead to the server responding with Rejected later down the line.

Here are the lines that I attempted to add

/* lib/request-base.js  */
  if (isObj && isObject(this._data)) {
    if (data.toJSON) data = data.toJSON(); // my added line (leads to failure later)
    for (var key in data) {
      this._data[key] = data[key];
    }
...
@niftylettuce
Copy link
Collaborator

PR welcome

@hasahmed
Copy link
Author

Yeah I tried to get it working, but it was quite in the weeds. When I finally figured out where it was should be called and added the code it caused an error. I'll post the error here if I ever get back around to checking it out again.

@niftylettuce
Copy link
Collaborator

@hasahmed I will release fix, one sec

@niftylettuce
Copy link
Collaborator

@hasahmed please try v5.0.9 released to npm, npm install superagent@latest

@MatthewHerbst
Copy link

MatthewHerbst commented Sep 25, 2019

Note: this change caused an unintentional library behavior change due to the new upstream dependency. Objects passed to superagent that have circular references no longer have their .toJSON method called if it exists. This means that if users of superagent were relying on this functionality (as we were), they now need to call the .serialize and manually call the .toJSON method there.

I have an open issue upstream related to this: davidmarkclements/fast-safe-stringify#46

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