firebase / firebase-functions

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

v2 Firestore Triggers Type definitions define .data property on FirestoreEvents as possibly undefined #1464

Closed wneild closed 1 month ago

wneild commented 12 months ago

Related issues

[REQUIRED] Version info

node: 16.20.0

firebase-functions: 4.4.1

firebase-tools: 12.3.0

firebase-admin: 11.10.1

[REQUIRED] Test case

export const exampleHandler = onDocumentCreated('example/{docId}', async (event) => {
    const docId = event.data?.id; // event data is possibly undefined
    info(docId);
});

[REQUIRED] Steps to reproduce

  1. Write the above code in a Node with Typescript configured environment
  2. Observe the need to handle possibly undefined event data

[REQUIRED] Expected behavior

I expect every Firestore event to, at minimum, have data on what occurred and never be undefined.

[REQUIRED] Actual behavior

I am required to handle the possibility of undefined data on all Firestore events as per the following type defs:

export declare function onDocumentWritten<Document extends string>(document: Document, handler: (event: FirestoreEvent<Change<DocumentSnapshot> | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<Change<DocumentSnapshot> | undefined, ParamsOf<Document>>>;

export declare function onDocumentWritten<Document extends string>(opts: DocumentOptions<Document>, handler: (event: FirestoreEvent<Change<DocumentSnapshot> | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<Change<DocumentSnapshot> | undefined, ParamsOf<Document>>>;

export declare function onDocumentCreated<Document extends string>(document: Document, handler: (event: FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>>;

export declare function onDocumentCreated<Document extends string>(opts: DocumentOptions<Document>, handler: (event: FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>>;

export declare function onDocumentUpdated<Document extends string>(document: Document, handler: (event: FirestoreEvent<Change<QueryDocumentSnapshot> | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<Change<QueryDocumentSnapshot> | undefined, ParamsOf<Document>>>;

export declare function onDocumentUpdated<Document extends string>(opts: DocumentOptions<Document>, handler: (event: FirestoreEvent<Change<QueryDocumentSnapshot> | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<Change<QueryDocumentSnapshot> | undefined, ParamsOf<Document>>>;

export declare function onDocumentDeleted<Document extends string>(document: Document, handler: (event: FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>>;

export declare function onDocumentDeleted<Document extends string>(opts: DocumentOptions<Document>, handler: (event: FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>) => any | Promise<any>): CloudFunction<FirestoreEvent<QueryDocumentSnapshot | undefined, ParamsOf<Document>>>;

Were you able to successfully deploy your functions?

Yes

google-oss-bot commented 12 months ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

johnnyoshika commented 9 months ago

I'm also interested in this? In what scenario would event.data be undefined?

Background: event for onDocumentWritten is typed as:

FirestoreEvent<Change<DocumentSnapshot> | undefined, ParamsOf<Document>>
exaby73 commented 8 months ago

There may be some documentation missing here. I'll label it as such

FelixLauwaert commented 3 months ago

I'm also having issues accessing any information.

const { onDocumentWritten } = require('firebase-functions/v2/firestore');
const { PubSub } = require('@google-cloud/pubsub');

const pubSubClient = new PubSub();
const TOPIC_NAME = 'foo';

exports.firestoreChangeListener = onDocumentWritten('bar/{docId}', async (event) => {

  try {
    const documentId = event.params.docId;
    const timestamp = event.timestamp;
    const beforeData = event.data.before.exists ? event.data.before.data() : null;
    const afterData = event.data.after.exists ? event.data.after.data() : null;
    const eventId = event.eventId;

    let changeType = '';
    if (!event.data.before.exists && event.data.after.exists) {
      changeType = 'created';
    } else if (event.data.before.exists && event.data.after.exists) {
      changeType = 'modified';
    } else if (event.data.before.exists && !event.data.after.exists) {
      changeType = 'deleted';
    }

    const messageData = {
      eventId,
      changeType,
      documentId,
      timestamp,
      afterData,
      beforeData
    };

    const messageBuffer = Buffer.from(JSON.stringify(messageData));
    await pubSubClient.topic(TOPIC_NAME).publish(messageBuffer);
    console.log('Message published to Pub/Sub successfully.');
  } catch (error) {
    console.error('Error publishing message to Pub/Sub:', error);
    throw error; // Rethrow the error to propagate it
  } 

});

Nothing actually exists. I'm able to send event as a whole and it's just a bunch of numbers. If I try with the lines above it fails with: Error publishing message to Pub/Sub: TypeError: Cannot read properties of undefined (reading 'before') at /workspace/index.js:12:35

taeold commented 2 months ago

Hey @johnnyoshika @wneild - thanks for the question!

After chatting with the team, I re-learned that the .data property may be empty if the snapshot for Firestore document being sent over the wire exceeds 10MB. This is very unlikely to happen, and when it happens you'll see a log in your GCP project.

See https://github.com/firebase/firebase-functions/pull/1370 for another description of the problem.

Surprisingly, I wasn't able to find the discussion of this limitation in public GCP docs - I'll see if this limitation still exists in the platform and try to find related documentation on it.

taeold commented 2 months ago

Ah the limitation is indirectly discussed here:

https://cloud.google.com/functions/quotas#resource_limits

Note that the behavior when the event is >10MB is "drop and log" - will ask if they plan on updating the doc to include that caveat.

exaby73 commented 1 month ago

Based on @taeold's comment above, this is working as intended. I'll close this issue