ladjs / superagent

Ajax for Node.js and browsers (JS HTTP client). Maintained for @forwardemail, @ladjs, @spamscanner, @breejs, @cabinjs, and @lassjs.
https://ladjs.github.io/superagent/
MIT License
16.57k stars 1.33k forks source link

[fix] seralization error of BigInt does not throw an exception #1769

Closed conneryn closed 10 months ago

conneryn commented 1 year ago

Describe the bug

Node.js version: v16.14.0 OS version: Ubuntu 18.04.6 LTS Description:

When attempting to send a POST request with a payload that contains a BigInt value, the JSON serialization silently fails and proceeds with the post request using a POST body set to: "[unable to serialize, circular reference is too complex to analyze]"

NOTE: I have not tested or checked if other serialization issues (ex: an actual circular reference) also have this same behaviour, but I imagine they very likely could.

Actual behavior

Superagent fails to serialize the BigInt to JSON, but instead of throwing an error, it simply continues to processes the POST request with the POST body set to: "[unable to serialize, circular reference is too complex to analyze]"

Expected behavior

Ideally, superagent would properly handle BigInt numbers... but at the very least it should:

  1. throw an exception when it fails to serialize data, and
  2. have an error message that is more accurate to the issue (ex: "Cannot serialize BigInt value" rather than a confusing message about circular references).

Code to reproduce

const superagent = require("superagent");
const data = {
    number: BigInt("1")
};

superagent
    .post("https://httpbin.org/post")
    .send(data)
    .end((err, res) => {
        console.log("ERROR:", err);
        console.log("POST BODY SENT TO SERVER:", res.body.data);
    });

Result:

ERROR: null
POST BODY SENT TO SERVER: "[unable to serialize, circular reference is too complex to analyze]"

Checklist

NikoRaisanen commented 11 months ago

Just put up a PR for that issue here https://github.com/ladjs/superagent/pull/1773

titanism commented 10 months ago

Closed per #1773

titanism commented 10 months ago

v8.1.0 released which fixes this issue, thank you

npm install superagent@8.1.0

release notes @ https://github.com/ladjs/superagent/releases/tag/v8.1.0