search

pulyaevskiy / firebase-admin-interop

Firebase Admin Interop Library for Dart
BSD 3-Clause "New" or "Revised" License
80 stars 37 forks source link

Issue with Firestore GeoPoint data #6

Closed lukef closed 6 years ago

lukef commented 6 years ago

Using the 1.0.0-dev (latest)

When I try to save GeoPoint data to Firestore I get the following error in the Firebase Functions log:

Error: Cannot encode type ([object Object]) to a Firestore Value
    at Function.encodeValue (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:775:11)
    at Function.encodeFields (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:646:36)
    at Function.fromObject (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:202:55)
    at WriteBatch.create (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/write-batch.js:178:39)
    at DocumentReference.create (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/reference.js:362:8)
    at CollectionReference.add (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/reference.js:2025:24)
    at UnknownJavaScriptObject.add$1 (/user_code/build/src/index.dart.js:818:25)
    at J.add$1$ax (/user_code/build/src/index.dart.js:14976:42)
    at CollectionReference.add$1 (/user_code/build/src/index.dart.js:13837:52)
    at $constructor.dart.pingHandler (/user_code/build/src/index.dart.js:14746:96)

Sample code:

  final document = new DocumentData();
  document.setGeoPoint("location", new GeoPoint(37.7991232,-122.4485953));

  final firestore = app.firebase.firestore();
  firestore
      .collection("test")
      .add(document)
      .then(allowInterop((_) {
        ...
      }));
pulyaevskiy commented 6 years ago

Thanks for the report! I do have a test case for GeoPoint type which worked initially. I'll try to reproduce it on my end and get back to you.

pulyaevskiy commented 6 years ago

It's working for me. Could you provide versions of Dart SDK and and Node's firebase libraries?

lukef commented 6 years ago

Yep! Let me know if you need more info. I'll also try to copy the test case to see if it's just how i'm adding the document.

Dart VM version: 2.0.0-dev.35.0 (Fri Mar 9 13:09:14 2018 +0100) on "windows_x64"
Node v9.8.0
Firebase Tools version 3.17.7
@firebase/app: 0.1.10
@firebase/app-types: 0.1.2
@firebase/util: 0.1.10
@firebase/auth: 0.3.4
@firebase/database: 0.1.11
@firebase/database-types: 0.1.2
@firebase/firestore: 0.3.5
@firebase/firestore-types: 0.2.2
pulyaevskiy commented 6 years ago

Also, note that you should never need to use allowInterop with this library, it's been designed to encapsulate these details. Your then call can just use regular Dart closure:

  final document = new DocumentData();
  document.setGeoPoint("location", new GeoPoint(37.7991232,-122.4485953));

  final firestore = app.firebase.firestore();
  firestore
      .collection("test")
      .add(document)
      .then((_) {
        // done
      });
pulyaevskiy commented 6 years ago

The only two packages required are firebase-admin and @google-cloud/firestore. Though I don't see either in your list, which is strange.

Could you double check that your package.json looks something like this?

{
    "dependencies": {
        "firebase-admin": "~5.8.1",
        "@google-cloud/firestore": "~0.11.1"
    }
}

I'll try to upgrade to latest (if there are newer versions) and run the tests again.

pulyaevskiy commented 6 years ago

It seems like the issue might be that you have dependency on @firebase/firestore while this library needs @google-cloud/firestore.

Honestly I'm not sure which one is supposed to be used.

PS: Upgraded to latest deps and everything seems working on my side still.

lukef commented 6 years ago

Sorry .. I was trying to be too fancy and pulling from the lock file.

My package.json is:

  "dependencies": {
    "firebase": "^4.11.0",
    "firebase-admin": "~5.8.1",
    "firebase-functions": "^0.8.1",
    "@google-cloud/firestore": "~0.13.0"
  },

I'm also using the firebase-functions-interop as well.

lukef commented 6 years ago

I swapped over to the following syntax and no change, sorry:

  final ref = firestore.document("test/1234");
  final document = new DocumentData.fromMap({
    'geoVal': new GeoPoint(23.03, 19.84),
  });
  ref.setData(document).then((_) {});
lukef commented 6 years ago

To be clear, it builds and deploys fine. The error will occur on execution. Execution takes a really long time but it might be because I'm not handling that error properly.

Function execution took 22763 ms, finished with status: 'crash'

Then the error above.

pulyaevskiy commented 6 years ago

Ok, was just able to reproduce with dependencies you provided. Trying to figure out where it goes wrong...

pulyaevskiy commented 6 years ago

One quick workaround that worked for me is to upgrade firebase-admin to latest. Looks like you have latest firestore package but stale firebase-admin. This package.json seems to be working for me:

{
  "dependencies": {
    "firebase": "^4.11.0",
    "firebase-admin": "~5.11.0",
    "firebase-functions": "^0.8.1",
    "@google-cloud/firestore": "~0.13.0"
  }
}

Could you verify if it works on your end?

lukef commented 6 years ago

Right now i'm getting:

Error: Argument "data" is not a valid Document. Detected an object of type "GeoPoint" that 
doesn't match the expected instance. Please ensure that the Firestore types you are using 
are from the same NPM package.

But i'm looking into it.

lukef commented 6 years ago

Full error is:


Error: Argument "data" is not a valid Document. Detected an object of type "GeoPoint" that 
doesn't match the expected instance. Please ensure that the Firestore types you are using are from 
the same NPM package.
    at Object.exports.(anonymous function) [as isDocument] (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/validate.js:86:15)
    at CollectionReference.add (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/reference.js:2147:14)
    at UnknownJavaScriptObject.add$1 (/user_code/build/src/index.dart.js:818:25)
    at J.add$1$ax (/user_code/build/src/index.dart.js:15173:42)
    at CollectionReference.add$1 (/user_code/build/src/index.dart.js:13975:52)
    at $constructor.dart.pingHandler (/user_code/build/src/index.dart.js:14943:96)
    at FirebaseApp_addHandler_closure.call$1 (/user_code/build/src/index.dart.js:13813:29)
    at HttpsFunctions_onRequest_jsHandler.dart.HttpsFunctions_onRequest_jsHandler.call$2 (/user_code/build/src/index.dart.js:14220:22)
    at Primitives_applyFunctionWithPositionalArguments (/user_code/build/src/index.dart.js:3077:28)
    at dart._callDartFunctionFast (/user_code/build/src/index.dart.js:13128:14)```
pulyaevskiy commented 6 years ago

Yeah :( We might need to stop using gcloud version of Firestore library. I'll try to get an update pushed sometime later today. A good news would be that we won't need an extra dependency in package.json though.

lukef commented 6 years ago

Sorry man. I'm in no rush for a fix today fwiw if you want to enjoy your Sunday :) Glad we got to the bottom of it at least. Thanks for following up!

pulyaevskiy commented 6 years ago

No problem, at all! Really appreciate you reporting this. Will update you here when I have something.

pulyaevskiy commented 6 years ago

Quick update. I see that firebase-admin is actually using @google-cloud/firestore under the hood, so we should be ok with keep using it.

However, when I use admin package locally there is no issues with using GeoPoint, only when used together with functions package on the server it breaks.

I'll probably have to try and build a JS-only version and see how it works.

pulyaevskiy commented 6 years ago

So, I think this has something to do with: http://justjs.com/posts/singletons-in-node-js-modules-cannot-be-trusted-or-why-you-can-t-just-do-var-foo-require-baz-init

I'll try to refactor Firestore bindings to not depend on requiring @google-cloud/firestore and see how it goes. Stay tuned.

pulyaevskiy commented 6 years ago

@lukef

I just published 1.0.0-dev.8.0. You should only need to run pub upgrade in your project to get this issue fixed.

I'd also recommend to adjust your package.json dependencies to use following constraints:

{
  "dependencies": {
    "firebase-admin": "5.10.0",
    "firebase-functions": "0.9.1",
    "@google-cloud/firestore": "0.13.0"
  }
}

Closing this issue for now. Feel free to re-open or create a new one if something is still not working.

lukef commented 6 years ago

Can confirm: IT WORKS! Thanks again for jumping on this quickly and thanks for the hard work.