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

Firestore emulator sending incorrect event payloads to Functions emulator #1288

Closed
samtstern opened this issue May 14, 2019 · 16 comments
Closed

Comments

@samtstern
Copy link
Contributor

samtstern commented May 14, 2019

I just ran a real Cloud Function (non-Firebase) triggered on onWrite for the document foo/bar.

I created the document (no previous data) and logged out the two arguments to the function and got this:

context

{
    eventId: "6e7a9aae-5e22-4414-abaf-cb6b0a3363a2-0",
    eventType: "providers/cloud.firestore/eventTypes/document.write",
    notSupported: {},
    resource: "projects/fir-dumpster/databases/(default)/documents/foo/bar",
    timestamp: "2019-05-14T01:49:26.587519Z",
}

data

{
    oldValue: {},
    updateMask: {},
    value: {
      createTime: "2019-05-14T01:49:26.587519Z",
      fields: {
        hey: {
          stringValue: "now",
        },
      },
      name: "projects/fir-dumpster/databases/(default)/documents/foo/bar",
      updateTime: "2019-05-14T01:49:26.587519Z",
    },
}

However if I do the same thing in the emulator suite I get:

context

{
  "eventId": "68ded781-de59-4adb-aff2-a8b6597a9725",
  "timestamp": "2019-05-14T01:59:17.900Z",
  "eventType": "providers/cloud.firestore/eventTypes/document.write",
  "resource": {
    "name": "projects/fir-dumpster/databases/(default)/documents/foo/bar",
    "service": "firestore.googleapis.com"
  }
}

data

{
  "value": {
    "name": "projects/fir-dumpster/databases/(default)/documents/foo/bar",
    "fields": {
      "hello": {
        "stringValue": "world"
      }
    },
    "createTime": "2019-05-14T01:59:17.900Z",
    "updateTime": "2019-05-14T01:59:17.900Z"
  },
  "updateMask": {}
}

The difference is that in the emulator we get context.resource is an Object and in prod we get context.resource is a String.

@samtstern
Copy link
Contributor Author

cc @abeisgoat

@samtstern
Copy link
Contributor Author

Note that I am working around this in #1260

@samtstern
Copy link
Contributor Author

I think there is something really funky going on here ... I can't find a single solution for resource assumptions that works for both RTDB and Firestore.

@samtstern
Copy link
Contributor Author

samtstern commented May 14, 2019

I checked for RTDB in prod and I got:

context

{
  "eventType":"providers/google.firebase.database/eventTypes/ref.write",
  "params":{},
  "auth":{"admin":true},
  "resource":"projects/_/instances/fir-dumpster/refs/foo/bar",
  "timestamp":"2019-05-14T18:49:56.923Z",
  "eventId":"BK6VnFw4oWjclYm+dJ1BE528dak="
}

data

{"data":null,"delta":{"hello":"world"}}

So it seems like resource should definitely be a string

@samtstern
Copy link
Contributor Author

samtstern commented May 14, 2019

https://github.com/firebase/firebase-functions/blob/18c01cbdc25d8f4fccb8847f6a4be86b6c7d4960/src/cloud-functions.ts#L47

/** Wire format for an event
 * @internal
 */
export interface Event {
  context: {
    eventId: string;
    timestamp: string;
    eventType: string;
    resource: Resource;
  };
  data: any;
}

https://github.com/firebase/firebase-functions/blob/18c01cbdc25d8f4fccb8847f6a4be86b6c7d4960/src/cloud-functions.ts#L160

/** Resource is a standard format for defining a resource (google.rpc.context.AttributeContext.Resource).
 * In Cloud Functions, it is the resource that triggered the function - such as a storage bucket.
 */
export interface Resource {
  service: string;
  name: string;
  type?: string;
  labels?: { [tag: string]: string };
}

https://github.com/firebase/firebase-functions/blob/18c01cbdc25d8f4fccb8847f6a4be86b6c7d4960/src/cloud-functions.ts#L64

/** The context in which an event occurred.
 * An EventContext describes:
 * - The time an event occurred.
 * - A unique identifier of the event.
 * - The resource on which the event occurred, if applicable.
 * - Authorization of the request that triggered the event, if applicable and available.
 */
export interface EventContext {
  /** ID of the event */
  eventId: string;
  /** Timestamp for when the event occured (ISO string) */
  timestamp: string;
  /** Type of event */
  eventType: string;
  /** Resource that triggered the event */
  resource: Resource;
  /** Key-value pairs that represent the values of wildcards in a database reference
   * Cannot be accessed while inside the handler namespace.
   */
  params: { [option: string]: any };
  /** Type of authentication for the triggering action, valid value are: 'ADMIN', 'USER',
   * 'UNAUTHENTICATED'. Only available for database functions.
   */
  authType?: 'ADMIN' | 'USER' | 'UNAUTHENTICATED';
  /** Firebase auth variable for the user whose action triggered the function. Field will be
   * null for unauthenticated users, and will not exist for admin users. Only available
   * for database functions.
   */
  auth?: {
    uid: string;
    token: object;
  };
}

@samtstern
Copy link
Contributor Author

samtstern commented May 14, 2019

Ah this line is interesting:
https://github.com/firebase/firebase-functions/blob/18c01cbdc25d8f4fccb8847f6a4be86b6c7d4960/src/cloud-functions.ts#L257

    if (legacyEventType && context.eventType === legacyEventType) {
      // v1beta1 event flow has different format for context, transform them to new format.
      context.eventType = provider + '.' + eventType;
      context.resource = {
        service: service,
        name: context.resource,
      };
    }

@abeisgoat
Copy link
Contributor

Yeah I think there's definitely multiple formats going around, I guess we just adhere to the newest?

@samtstern
Copy link
Contributor Author

Ok I have this in a place where everything works, but I feel like it shouldnt work and that's what scares me.

@samtstern
Copy link
Contributor Author

samtstern commented May 14, 2019

Here's what a Firestore update data payload looks like:

 {
  "oldValue": {},
  "updateMask": {},
  "value": {
    "createTime": "2019-05-14T23:04:30.459119Z",
    "fields": {
      "new": {
        "stringValue": "doc"
      }
    },
    "name": "projects/fir-dumpster/databases/(default)/documents/foo/bar",
    "updateTime": "2019-05-14T23:04:30.459119Z"
  }
}

I can't see how this is valid without oldValue having name and readTime ...

the onWrite handler seems to invoke changeConstructor which calls beforeSnapshotConstructor which requires oldValue stuff:
https://github.com/firebase/firebase-functions/blob/18c01cbdc25d8f4fccb8847f6a4be86b6c7d4960/src/providers/firestore.ts#L160

@laurenzlong
Copy link
Contributor

laurenzlong commented May 14, 2019

Basically there are 2 versions of EventFlow (the event delivery system). Some event providers have migrate and others have not, that's why there are inconsistencies in "resource" format.

To make things easier for our customers, the firebase-functions SDK converts everything into the new format. It seems that the emulator suite also adopts the new format. However if you create functions without the firebase-functions SDK, there are variations in prod.

@laurenzlong
Copy link
Contributor

The firebase-functions SDK also does transformations on the data payload of Firestore-triggered functions and turns raw JSON payload (which includes fields like oldValue and updateMask) into an easier to use Firestore snapshot. See https://github.com/firebase/firebase-functions/blob/master/src/providers/firestore.ts#L145-L180

@abeisgoat
Copy link
Contributor

@samtstern Is this still relevant after your merge yesterday?

@samtstern
Copy link
Contributor Author

@abeisgoat yes this will be fixed when @yuchenshi moves the emulator to v1beta1 protos.

@mhuebert
Copy link

I found this issue via https://stackoverflow.com/questions/56587770/firebase-functionsshell-onwrite-property-undefined - but I'm not sure if it is precisely the same issue, or not, so making a quick note here.

I am not able to run RTDB functions in the shell, context is undefined on this line:

if (legacyEventType && context.eventType === legacyEventType) {

(see https://github.com/firebase/firebase-functions/blob/v3.0.1/src/cloud-functions.ts#L243)

@samtstern
Copy link
Contributor Author

@mhuebert if you're using firebase-tools 7+ and firebase-functions 3+ that is a known issue at the moment. Downloading either the CLI to 6.11.0 or firebase-functions to 2.3.1 will solve your issue for now.

@samtstern
Copy link
Contributor Author

I don't actually think this is a problem anymore. It would be more correct for the emulator to send the other proto format, but we're papering over this fine. Closing this since we're not working on it.

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

No branches or pull requests

5 participants