firebase / firebase-functions

Firebase SDK for Cloud Functions
https://firebase.google.com/docs/functions/
MIT License
1.03k stars 203 forks source link

Firestore trigger: event.data.data() throws error when parsing GeoPoint(0, 0) #180

Closed k0ff33 closed 5 years ago

k0ff33 commented 6 years ago

Version info

firebase-functions: 0.8.1

firebase-tools: 3.17.4

firebase-admin: 5.8.2

Steps to reproduce

  1. Subscribe firebase function to certain document in Cloud Firestore
  2. Add an object with a GeoPoint with latitude of 0 and longitude of 0 using: new firebase.firestore.GeoPoint(0, 0) (web SDK)
  3. Try to parse event's data using event.data.data()
  4. Watch the error in the console

Were you able to successfully deploy your functions?

No errors, successfully deployed.

Expected behavior

Function should parse the data to a valid GeoPoint object with _latitude of 0 and longitude of 0. Similar situation with new firebase.firestore.GeoPoint(1, 1) works perfectly fine.

Actual behavior

 Error: Argument "latitude" is not a valid number.
    at Object.exports.(anonymous function) [as isNumber] (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/validate.js:86:15)
    at new GeoPoint (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:95:14)
    at Function.fromProto (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:151:12)
    at QueryDocumentSnapshot._decodeValue (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:565:25)
    at QueryDocumentSnapshot.data (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:430:26)
    at QueryDocumentSnapshot.data (/user_code/node_modules/firebase-admin/node_modules/@google-cloud/firestore/src/document.js:871:22)
    at Object.<anonymous> (/user_code/lib/firestore/newPostListener.js:19:42)
    at next (native)
    at /user_code/lib/firestore/newPostListener.js:7:71
    at __awaiter (/user_code/lib/firestore/newPostListener.js:3:12)
    at exports.default (/user_code/lib/firestore/newPostListener.js:17:30)
    at Object.<anonymous> (/user_code/node_modules/firebase-functions/lib/cloud-functions.js:59:27)
    at next (native)
    at /user_code/node_modules/firebase-functions/lib/cloud-functions.js:28:71
    at __awaiter (/user_code/node_modules/firebase-functions/lib/cloud-functions.js:24:12)
    at cloudFunction (/user_code/node_modules/firebase-functions/lib/cloud-functions.js:53:36)

Not working example with GeoPoint(0, ):

screen shot 2018-02-05 at 16 51 32

Working example with GeoPoint(1, 1):

screen shot 2018-02-05 at 17 02 33
choipd commented 6 years ago

I've also experienced this issue, too. And I decided to remove geo location info when user dose not provide it, rather than setting GeoPoint(0, 0). Because it is more natural.

Anyhow IMHO this issue is bug. There shouldn't have exception for GeoPoint(0,0).

thechenky commented 5 years ago

@k0ff33 Can you try this with the latest version of firebase-functions?

joehan commented 5 years ago

@thechenky Tested this with the latest version of firebase-functions (v2.1.0). The API has been changed since the version used in the question, from onWrite((event)=>{}) to onWrite((change, context)=>{}). However, I was able to parse GeoPoint(0,0) using change.after.data(). screenshot from 2019-01-02 15-08-34

thechenky commented 5 years ago

@joehan thanks for the update! @k0ff33 please update to the latest version of firebase-functions as this has been fixed.

thechenky commented 5 years ago

Closing this out.