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

Firebase Emulator error when writing/updating document inside a Firestore trigger, when using snapshot.ref.set() #1682

Closed
ralcar opened this issue Sep 30, 2019 · 12 comments · Fixed by #1714

Comments

@ralcar
Copy link

ralcar commented Sep 30, 2019

[REQUIRED] Environment info

firebase-tools:
7.4.0

Platform:
Windows

[REQUIRED] Test case

Since auth is not available in the context for Firestore triggers, I send along some user data in the snapshot to log the activity and use the user token to create some extra steps inside the onCreate function. I extract the userdata from the snapshot and the want to remove the userdata using the snapshot.ref.set(snapShotWithoutUserData)

[REQUIRED] Steps to reproduce

export const onOrganizerCreate = functions.firestore.document("organizers/{organizerId}")
.onCreate((snapshot, context) => {
  return snapshot.ref.set({
    test: "testing"
  }, { merge: true });
})

I have tried ref.set(), ref.update() with or without the merge parameter, same result

[REQUIRED] Expected behavior

Expect it to run without errors as it does when i deploy it.

[REQUIRED] Actual behavior

The emulator/trigger crashes, giving this error

If i deploy the same code and run it without the emulator, it works just fine. Or if i use the firebase-admin to do the same task by building the path using the context params, it also works

! Non-default "firebase-admin" instance created!

  • This instance will not be mocked and will access production resources.
    ! Google API requested!
  • URL: "https://oauth2.googleapis.com/token"
  • Be careful, this may be a production service.
    ! functions: Error: Getting metadata from plugin failed with error: invalid_grant
    at Http2CallStream.call.on (C:\Development<projectId>\functions\node_modules@grpc\grpc-js\build\src\client.js:96:45)
    at Http2CallStream.emit (events.js:194:15)
    at Http2CallStream.EventEmitter.emit (domain.js:441:20)
    at process.nextTick (C:\Development<projectId>\functions\node_modules@grpc\grpc-js\build\src\call-stream.js:71:22)
    at process._tickCallback (internal/process/next_tick.js:61:11)
    ! Your function was killed because it raised an unhandled error.
    (node:13324) UnhandledPromiseRejectionWarning: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are
    sent to the client
    at ServerResponse.setHeader (_http_outgoing.js:470:11)
    at ServerResponse.header (C:\Users\ralcar\AppData\Roaming\npm\node_modules\firebase-tools\node_modules\express\lib\response.js:771:10)
    at ServerResponse.send (C:\Users\ralcar\AppData\Roaming\npm\node_modules\firebase-tools\node_modules\expressginated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
@ralcar ralcar changed the title Writing/updating to snapshot.ref in Firebase Emulator fails Firebase Emulator error when writing/updating document inside a Firestore trigger, when using snapshot.ref.set() Sep 30, 2019
@samtstern samtstern self-assigned this Sep 30, 2019
@samtstern
Copy link
Contributor

samtstern commented Sep 30, 2019

@ralcar thanks that's an interesting bug, I think I can see how it happened.

For now you can workaround this by doing:

const newRef = admin.firestore().doc(snapshot.ref.path);
return newRef.set({ ... });

This will construct a new reference using the emulated admin.firestore().

@samtstern
Copy link
Contributor

Note to self:
https://github.com/firebase/firebase-functions/blob/master/src/apps.ts#L107

I think we could get around this by initializing an app with the name __admin__ in the emulators as a way of tricking the firebase-functions library to use our mocks. It would be relying on an internal implementation detail but I don't see how it would be harmful.

@ralcar
Copy link
Author

ralcar commented Sep 30, 2019

Thank you for the quick answer, that worked great, thank you! Was worried i was going to have to build that path everywhere. This is cleaner, but I hope I could avoid importing the firebase-admin at all in the future ;).

Just for some more help with the debugging, i initializeApp on two different databases during bootup of the cloud functions. And i am locally on Node 10.15.1

@samtstern
Copy link
Contributor

@ralcar could you show the code where you initializeApp twice? It would be helpful so that I can recreate the situation.

@larssn
Copy link

larssn commented Sep 30, 2019

I saw something similar using snapshot.ref.update() (also inside a trigger), which claimed the document didn't exist. Maybe related.

Weirdly enough though, snapshot.ref.set(doc, {merge: true}); worked without errors.

@samtstern
Copy link
Contributor

samtstern commented Sep 30, 2019

@larssn that sounds correct. .update() operations fail if the document does not exist. .set() operations create the document if it doesn't exist. set(data, { merge: true }) on a non-existent document is identical to set(data).

@larssn
Copy link

larssn commented Sep 30, 2019

Sorry, I should have mentioned what kind of trigger it was, as it was the onCreate trigger.
The document should exist at that point I take it? In any case, it was easily solved using set.

@ralcar
Copy link
Author

ralcar commented Sep 30, 2019

@samtstern ofcourse, here it is!
My /functions/src/index.ts loads "main.ts" first, that looks like this:

import { config } from 'firebase-functions';
import { initializeApp, credential } from 'firebase-admin';
initializeApp(config().firebase);

const productionServiceAccount = require("../<production>.json");
const stagingServiceAccount = require("../<staging>.json");

initializeApp({
  credential: credential.cert(productionServiceAccount),
  databaseURL: "<url>"
}, "production");

initializeApp({
  credential: credential.cert(stagingServiceAccount),
  databaseURL: "<url>"
}, "staging");

export * from 'firebase-functions';

@samtstern
Copy link
Contributor

@ralcar thanks!

FYI initializeApp(config().firebase) is outdated, you should just use initializeApp() ... that doesn't change the bug here but it's worth noting.

@samtstern
Copy link
Contributor

@larssn yes the doc should definitely exist at this point. I worry that your code is talking to prod sometimes ... changing it to set() may have fixed the symptom but covered up the bug.

@samtstern
Copy link
Contributor

Working on this over here:
firebase/firebase-functions#565

@samtstern
Copy link
Contributor

The fix for this issue has been released in version 7.6.0

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

Successfully merging a pull request may close this issue.

3 participants