NaN Double values are not handled, which results in an error parsing Firestore Documents

[rrousselGit]

Original: invertase/dart_firebase_apis#7

To put it simply, this document:

produces the following error in the conversion:

I can reproduce and more than willing to hop on a call to test it.
Im not versed in code generation so I'd appreceate someone stepping in with a PR.

Original Issue in Firebase Admin SDK:
invertase/dart_firebase_admin#29

See also Firestore official docs mentioning NaN values:
https://firebase.google.com/docs/firestore/manage-data/data-types

[rrousselGit]

cc @markbreuss

[rakudrama]

It appears that NaN values are passed in the 'JSON' as the string "NaN", as JSON itself is not capable of representing a NaN value.

The generator would need to be changed to handle this case, in both directions.

[kevmoo]

Just need to put a converter on the fields here. This is a convention we likely won't support directly.

[kevmoo]

Oops! We're not looking at JsonSerialiable

[kevmoo]

This looks like a bug in the JSON encoding of these values. There is no NaN supported. The defined type is double.

See https://developers.google.com/discovery/v1/type-format

[rakudrama]

I would suggest changing the generator to use helper methods for doubles.

With the extension below you can change the generator to switch from

doubleValue: json_.containsKey('doubleValue')
              ? (json_['doubleValue'] as core.num).toDouble()
              : null,
...
     {...
       if (doubleValue != null) 'doubleValue': doubleValue!,

to

doubleValue: json_.doublePropertyOrNull('doubleValue')

...
    {...
      if (doubleValue != null) 'doubleValue': JsonHelp.encodeDouble(doubleValue!),

The overall generated code will be smaller if you also use extension methods for parsing other fields (e.g. boolProperty)
At the lowest levels, json_['boolValue'] as bool is effectively two calls - an indexer and a check on the type. The suggested replacement of json_.boolProperty('boolValue') is a single call.

A final trick is that you can use an extension method with null-aware operators to call a constructor via its tearoff, so

          dateValue: json_.containsKey('dateValue')
              ? Date.fromJson(
                  json_['dateValue'] as core.Map<core.String, core.dynamic>)
              : null,

becomes

          dateValue: (json_['dateValue'] as core.Map?)?.convertUsing(Date.fromJson),

---

extension JsonHelp on Map {
  double doubleProperty(String key) {
    final value = this[key];
    if (value == null && !this.containsKey(key)) throw _missingPropery(key);
    return _toDouble(key, value);
  }

  double? doublePropertyOrNull(String key) {
    final value = this[key];
    if (value == null && !this.containsKey(key)) return null;
    return _toDouble(key, value);
  }

  static double _toDouble(String key, Object? value) {
    if (value is num) return value.toDouble();
    if (value is String) {
      if ('NaN' == value) return double.nan;
      if ('Infinity' == value) return double.infinity;
      if ('-Infinity' == value) return double.negativeInfinity;
    }
    throw ArgumentError.value(
        value, key, "Is not a number, 'NaN', 'Infinity', or '-Infinity'");
  }

  Error _missingPropery(String key) {
    return ArgumentError.value(this, 'json', "Map is missing property '$key'");
  }

  static Object encodeDouble(double value) {
    if (value.isFinite) return value;
    if (value.isNaN) return 'NaN';
    return value < 0 ? '-Infinity' : 'Infinity';
  }
  
  /// Simple 
  bool boolProperty(String key) {
    final value = this[key];
    if (value == null && !this.containsKey(key)) throw _missingPropery(key);
    return value as bool;
  }

  bool? boolPropertyOrNull(String key) {
    final value = this[key];
    if (value == null && !this.containsKey(key)) return null;
    return value as bool;
  }


  
  T convertUsing<T>(T Function(Map) convert) {
    return convert(this);
  }

}

[rrousselGit]

@kevmoo The JS SDK doesn't seem to do anything special with NaN and sends it as double though
https://github.com/googleapis/nodejs-firestore/blob/f73e28b69be3ff9f870055af80683f12048549e8/dev/src/serializer.ts#L125

[kevmoo]

@rrousselGit – would you send me the full stacktrace?

We're looking at https://github.com/invertase/dart_firebase_apis/blob/main/generated/firebaseapis/lib/firestore/v1.dart right?

[rrousselGit]

Correct.
To be specific, it's used through https://github.com/invertase/dart_firebase_admin/

Here's a test:

void main() {
    test('Supports nan as double value', () async {
      final doc = firestore.collection('foo').doc('123');

      await doc.create({'value': double.nan});
    });
}

It fails with the following stacktrace:

00:01 +127 -2: test/google_cloud_firestore/collection_group_test.dart: collectionGroup Supports nan as double value [E]
  Converting object to an encodable object failed: Instance of 'CommitRequest'
  dart:convert                                                                    JsonCodec.encode
  package:firebaseapis/firestore/v1.dart 1221:32                                  ProjectsDatabasesDocumentsResource.commit
  package:dart_firebase_admin/src/google_cloud_firestore/write_batch.dart 116:50  WriteBatch._commit.<fn>
  package:dart_firebase_admin/src/google_cloud_firestore/firestore.dart 239:21    _FirestoreHttpClient.v1.<fn>
  ===== asynchronous gap ===========================
  package:dart_firebase_admin/src/google_cloud_firestore/write_batch.dart 92:22   WriteBatch.commit
  ===== asynchronous gap ===========================
  package:dart_firebase_admin/src/google_cloud_firestore/reference.dart 282:21    DocumentReference.create
  ===== asynchronous gap ===========================
  test/google_cloud_firestore/collection_group_test.dart 50:7                     main.<fn>.<fn>

[kevmoo]

Right. Because JSON doesn't support it! https://www.json.org/

[rrousselGit]

Sure. But Firestore does support NaN values, doesn't it?
https://cloud.google.com/firestore/docs/concepts/data-types

Afaik on mobile, the equivalent of Value is sent as {isNan: true}. There's probably something similar here?

[rrousselGit]

Here's a FlutterFire test that sets a document field to nan: https://github.com/firebase/flutterfire/blob/c7963d63b1cd8cf6471959f0ee7fbf45b5f51edc/packages/cloud_firestore/cloud_firestore/example/integration_test/document_reference_e2e.dart#L388

[kevmoo]

Sure! I get all of those. But we're talking about the JSON endpoints. These are auto-generated from the discovery documents. I could one-off hack these, it's just annoying.

…

On Mon, Jul 22, 2024 at 2:46 PM Remi Rousselet ***@***.***> wrote: Here's a FlutterFire test that sets a document field to nan: https://github.com/firebase/flutterfire/blob/c7963d63b1cd8cf6471959f0ee7fbf45b5f51edc/packages/cloud_firestore/cloud_firestore/example/integration_test/document_reference_e2e.dart#L388 — Reply to this email directly, view it on GitHub <#638 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEFCR6253MPL2ABWAHJVLZNV4TJAVCNFSM6AAAAABK4XW74OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBTHA3DQNZWGM> . You are receiving this because you were mentioned.Message ID: <google/googleapis.dart/issues/638/2243868763 ***@***.*** com>