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

BigInt serialization error on Integer and Null union type #501

Open
2 tasks done
fauh45 opened this issue Aug 3, 2022 · 11 comments
Open
2 tasks done

BigInt serialization error on Integer and Null union type #501

fauh45 opened this issue Aug 3, 2022 · 11 comments
Labels
bug Confirmed bug

Comments

@fauh45
Copy link

fauh45 commented Aug 3, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.3.0

Plugin version

No response

Node.js version

16.16.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

elementary OS 6.1 Jólnir (Ubuntu 20.04.3 LTS)

Description

Using typebox as the schema generator, whenever I made a type on the schema where it is a union of Integer and Null, Fastify always return as such.

{
  "statusCode": 500,
  "error": "Internal Server Error",
  "message": "Do not know how to serialize a BigInt"
}

With the stacktrace of,

TypeError: Do not know how to serialize a BigInt
              at JSON.stringify (<anonymous>)
              at anonymous12 (eval at build (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fast-json-stringify/index.js:177:23), <anonymous>:76:50)
              at main (eval at build (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fast-json-stringify/index.js:177:23), <anonymous>:6:15)
              at serialize (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fastify/lib/reply.js:797:12)
              at preserializeHookEnd (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fastify/lib/reply.js:478:17)
              at preserializeHook (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fastify/lib/reply.js:462:5)
              at Reply.send (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fastify/lib/reply.js:182:7)
              at Object.<anonymous> (/home/xxxxxx/xxxxxx/xxxxxx/index.ts:74:30)
              at preHandlerCallback (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fastify/lib/handleRequest.js:126:28)
              at preValidationCallback (/home/xxxxxx/xxxxxx/xxxxxx/node_modules/fastify/lib/handleRequest.js:110:5)

Steps to Reproduce

  1. Create a endpoint (using Typebox v0.24.20) as such

    app.get(
    "/",
    {
        schema: {
          response: {
            "200": Type.Object({
              test2: Type.Union([Type.Integer(), Type.Null()]),
            }),
          },
        },
      },
      async (req, res) => {
        return res.status(200).send({
          // @ts-expect-error No BigInt Type in Typebox
          test2: 12n,
        });
      }
    );
  2. Call the endpoint

Expected Behavior

The serializer should serialize as an integer if it's an BigInt, and null if it's not

@climba03003
Copy link
Member

I think the problem based on ajv do not know how to deal with BigInt.

@fauh45
Copy link
Author

fauh45 commented Aug 3, 2022

I think the problem based on ajv do not know how to deal with BigInt.

Isn't there a BigInt serializer to Number on fastify-json-stringify though?

@climba03003
Copy link
Member

Isn't there a BigInt serializer to Number on fastify-json-stringify though?

When using anyOf (Type.Union), the data will validate through ajv first.

@fauh45
Copy link
Author

fauh45 commented Aug 3, 2022

When using anyOf (Type.Union), the data will validate through ajv first.

Ahhh I see how it is, so the culprit here are ajv

@climba03003 climba03003 added the bug Confirmed bug label Aug 3, 2022
@mcollina
Copy link
Member

mcollina commented Aug 3, 2022

I don't think we can ever fix this without some Ajv support. Maybe we should document this caveat?

@ivan-tymoshenko
Copy link
Member

@mcollina I guess we can do the same thing as we have done for js Date object #441.

@mcollina
Copy link
Member

mcollina commented Aug 3, 2022

Actually yes, we can!

@ivan-tymoshenko
Copy link
Member

ivan-tymoshenko commented Aug 3, 2022

Actually, I was about to write a post/issue about the whole validation situation. In short, we use the same syntax of schema to describe how to serialize data (with type coercion, it's important) and to validate the same data. Here we have some inconsistency problems: date type (already fixed), bigint, regexp, etc.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 3, 2022

Can you make a list of needed data type validators?

@ivan-tymoshenko
Copy link
Member

I have carefully studied this topic. Ajv does not support bigint type or bigint cast. It also cannot be resolved in the way we did for the Date type, because the Date type is the union of existing in Ajv types: string and date, while the bigint type is not a number or an integer.

This can be solved by specifying your own Ajv keyword. All occurrences of integer in user schemas will need to be replaced with this keyword. Support for integer formats will not automatically work with this keyword.

I don't know of a simple solution to this problem right now.

I leave here a test for anyone who wants to try to solve this problem.

test('anyOf with bigint type', (t) => {
  t.plan(1)

  const schema = {
    type: 'object',
    properties: {
      id: {
        anyOf: [
          {
            type: 'integer'
          },
          {
            type: 'null'
          }
        ]
      }
    }
  }

  const stringify = build(schema)

  const data = { id: 12n }
  const output = stringify(data)

  t.equal(output, '{"id":12}')
})

Ajv issue about that: ajv-validator/ajv#1116

@hazer-hazer
Copy link

Hi.
I've struggled with this so much, I tried to fix this.
At first, I'd say that if we serialize bigint as string then we pass the check for bigint against the string, and the fix is not that complex, the user already expects that bigint becomes string so why doesn't bigint == string in terms of this library? But if we dive so deep about fast-json-stringify, we'll see that AJV should not ever be used (no calls such as if validator.validate), I mean, each value should be checked and then serialized by rules defined in fjs.

Anyway.

The first thing I got to modify is multiple types (such as ["string", "number"]).
It works pretty well, but it does not resolve this issue fully, bigint in multiple types is now only supported for array syntax.
But when I moved to anyOf/oneOf, I got that we either do all AJV validations by ourselves and serialize if the value is valid or ignore some validation properties.
It's funny logic user does not expect:

// Works, because `pattern` is ignored, AJV is unused.
build({
    type: 'string',
    pattern: '[A-Z]+',
}).stringify('123') === '"123"'

// Throws an error, because `anyOf` does validation call to AJV.
build({
    anyOf: [{
         type: 'string',
         pattern: '[A-Z]+'
    }]
}).stringify('123') === '"123"'

If AJV is used only "sometimes", I'd consider a solution where it's unused in the serialization flow graph at all, and FJS only supports a subset of validation-only (format, pattern, etc.) AJV keywords.
There're two ways I see:

  1. Fast JSON stringify
  • Don't do primitive types validation (such as checking for pattern)
  • Serialization won't be precise, fjs already isn't (example with pattern in anyOf above). And it's gonna be hard to deal with complex anyOfs.
  • AJV will never be called twice (it happens in case of nested anyOf)
  1. Typed JSON stringify
  • Fork some latest stable AJV version and add serialization beside validation
  • Write new validator-serializer from scratch.
  1. Replace calls to validator.validate in anyOf with try-catch serialize. But now might not be fast-json-stringify.

Now I'd like to explain for people struggling with this problem why it even exists.

  1. In simple cases FJS works by trying to serialize your value of type T to type ascribed in schema Ts. For example, schema {type: 'string'} will serialize your number 123 as a string "123".
  2. But when we use anyOf or oneOf, FJS generates if-else statements with condition on AJV validation. So before your value is serialized, it is checked against AJV schema, and 123n (bigint) won't ever be valid string or integer or anything else because AJV doesn't know about bigint.

Excuse me for my crooked english, I hope this text sounds sensibly.

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

No branches or pull requests

6 participants