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

Improve error message when invalid Firestore document path is specified in Firestore trigger definitions #4284

Open
evengul opened this issue Mar 10, 2022 · 7 comments

Comments

@evengul
Copy link

evengul commented Mar 10, 2022

Wasn't really sure what to label this, so feel free to change it to something clearer.

Related issues

Related, closed issue from 2018 fixed by fix of bug in grpc: firebase/firebase-functions#536 .

[REQUIRED] Version info

Versions:

node: 16.13.0

firebase-functions: 3.18.1

firebase-tools: 10.2.2 (Also happened with 10.1.2)

firebase-admin: 10.0.2

[REQUIRED] Test case

index.ts (looks mostly the same transpiled).

import * as admin from 'firebase-admin';
import * as functions from 'firebase-functions';

const app = admin.apps[0] ?? admin.initializeApp();

const testFunction = functions.https.onRequest(async (req, res) => {
  try {
    await app.firestore().collection('test').doc('test').set({ test: true });
    res.send(true);
  } catch (e) {
    console.error(e);
    res.send(false);
  }
});

export default testFunction;

[REQUIRED] Steps to reproduce

  1. Compile the typescript
  2. Run the function on the emulator using firebase emulators:start.
  3. Send a request to http://localhost:5001/{projectId}/{location}/default
  4. See in the console that we get an error with the note "Exception occurred in retry method that was not classified as transient". Complete error message below.

[REQUIRED] Expected behavior

The collection "test" should have a document "test" with a value "test":true in it.

[REQUIRED] Actual behavior

Sporadically, it seems to work, this might have something to do with cold starts? I don't know how that works on the emulator.

The logged exception:

Error: 2 UNKNOWN: 
>      at Object.callErrorFromStatus (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\@grpc\grpc-js\build\src\call.js:31:26)
>      at Object.onReceiveStatus (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\@grpc\grpc-js\build\src\client.js:180:52)
>      at Object.onReceiveStatus (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\@grpc\grpc-js\build\src\client-interceptors.js:365:141)
>      at Object.onReceiveStatus (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\@grpc\grpc-js\build\src\client-interceptors.js:328:181)
>      at C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\@grpc\grpc-js\build\src\call-stream.js:182:78
>      at processTicksAndRejections (node:internal/process/task_queues:78:11)
>  Caused by: Error
>      at WriteBatch.commit (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\@google-cloud\firestore\build\src\write-batch.js:417:23)
>      at DocumentReference.set (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\@google-cloud\firestore\build\src\reference.js:355:14)
>      at C:\Users\Even\dev\flatz\app-2.0\functions\lib\index.js:9:62
>      at C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\firebase-tools\lib\emulator\functionsEmulatorRuntime.js:574:16
>      at runFunction (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\firebase-tools\lib\emulator\functionsEmulatorRuntime.js:547:15)
>      at runHTTPS (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\firebase-tools\lib\emulator\functionsEmulatorRuntime.js:573:11)
>      at handler (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\firebase-tools\lib\emulator\functionsEmulatorRuntime.js:493:23)
>      at Layer.handle [as handle_request] (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\firebase-tools\node_modules\express\lib\router\layer.js:95:5)
>      at next (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\firebase-tools\node_modules\express\lib\router\route.js:137:13)
>      at Route.dispatch (C:\Users\Even\dev\flatz\app-2.0\functions\node_modules\firebase-tools\node_modules\express\lib\router\route.js:112:3) {
>    code: 2,
>    details: '',
>    metadata: Metadata {
>      internalRepr: Map(1) { 'content-type' => [Array] },
>      options: {}
>    },
>    note: 'Exception occurred in retry method that was not classified as transient'

The error message showing up in firestore-debug.log:

Mar 10, 2022 7:58:49 PM com.google.cloud.datastore.emulator.impl.util.WrappedStreamObserver onError
INFO: operation failed: Invalid pattern. Reason: [76:<eof>] Expected '}' at end of capture expression.
java.lang.IllegalArgumentException: Invalid pattern. Reason: [76:<eof>] Expected '}' at end of capture expression.
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:147)
	at com.google.firebase.rules.eventflow.client.path.PathPattern.compileInternal(PathPattern.java:78)
	at com.google.firebase.rules.eventflow.client.path.PathPattern.compile(PathPattern.java:50)
	at com.google.firebase.rules.eventflow.client.EventRuleEvaluatorImpl.lambda$new$0(EventRuleEvaluatorImpl.java:58)
	at com.google.common.collect.ImmutableList.forEach(ImmutableList.java:447)
	at com.google.firebase.rules.eventflow.client.EventRuleEvaluatorImpl.<init>(EventRuleEvaluatorImpl.java:52)
	at com.google.firebase.rules.eventflow.client.EventRuleEvaluatorImpl.<init>(EventRuleEvaluatorImpl.java:39)
	at com.google.cloud.datastore.emulator.impl.events.EventManager.reportEvents(EventManager.java:84)
	at com.google.cloud.datastore.emulator.impl.CloudFirestoreV1.commitHelper(CloudFirestoreV1.java:891)
	at com.google.cloud.datastore.emulator.impl.CloudFirestoreV1.internalCommit(CloudFirestoreV1.java:780)
	at com.google.cloud.datastore.emulator.impl.CloudFirestoreV1.commit(CloudFirestoreV1.java:416)
	at com.google.cloud.datastore.emulator.impl.CloudFirestoreV1Router.commit(CloudFirestoreV1Router.java:137)
	at com.google.cloud.datastore.emulator.firestore.v1.FirestoreV1GrpcAdapter$1.lambda$commit$8(FirestoreV1GrpcAdapter.java:156)
	at com.google.cloud.datastore.emulator.firestore.v1.FirestoreV1GrpcAdapter.unary(FirestoreV1GrpcAdapter.java:67)
	at com.google.cloud.datastore.emulator.firestore.v1.FirestoreV1GrpcAdapter.access$000(FirestoreV1GrpcAdapter.java:39)
	at com.google.cloud.datastore.emulator.firestore.v1.FirestoreV1GrpcAdapter$1.commit(FirestoreV1GrpcAdapter.java:156)
	at com.google.firestore.v1.FirestoreGrpc$MethodHandlers.invoke(FirestoreGrpc.java:1224)
	at io.grpc.stub.ServerCalls$UnaryServerCallHandler$UnaryServerCallListener.onHalfClose(ServerCalls.java:182)
	at io.grpc.PartialForwardingServerCallListener.onHalfClose(PartialForwardingServerCallListener.java:35)
	at io.grpc.ForwardingServerCallListener.onHalfClose(ForwardingServerCallListener.java:23)
	at io.grpc.ForwardingServerCallListener$SimpleForwardingServerCallListener.onHalfClose(ForwardingServerCallListener.java:40)
	at io.grpc.Contexts$ContextualizedServerCallListener.onHalfClose(Contexts.java:86)
	at io.grpc.PartialForwardingServerCallListener.onHalfClose(PartialForwardingServerCallListener.java:35)
	at io.grpc.ForwardingServerCallListener.onHalfClose(ForwardingServerCallListener.java:23)
	at io.grpc.ForwardingServerCallListener$SimpleForwardingServerCallListener.onHalfClose(ForwardingServerCallListener.java:40)
	at io.grpc.Contexts$ContextualizedServerCallListener.onHalfClose(Contexts.java:86)
	at io.grpc.internal.ServerCallImpl$ServerStreamListenerImpl.halfClosed(ServerCallImpl.java:331)
	at io.grpc.internal.ServerImpl$JumpToApplicationThreadServerStreamListener$1HalfClosed.runInContext(ServerImpl.java:866)
	at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
	at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:133)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

I can see that it expects an ending bracket }, but I can't figure out where it expects it.

Were you able to successfully deploy your functions?

Don't know if this is relevant, when I can't run them locally.

I've also attached my complete logs, but I'm pretty sure I've pasted the relevant parts.
firebase-debug.log
firestore-debug.log

@evengul
Copy link
Author

evengul commented Mar 10, 2022

I upgraded to firebase-tools@10.2.2 yesterday. Downgraded back to 10.1.1 now, and it looks to be working. Is there an issue with the newest version(s)?

Edit: No, that wasn't it. When I rebuilt with another function and a new function was found, the first function failed again.

@taeold
Copy link
Contributor

taeold commented Mar 10, 2022

Tentaively moving the issue to Firebase CLI repo - it looks like the bug might be w/ the Firestore Emulator.

@taeold taeold transferred this issue from firebase/firebase-functions Mar 10, 2022
@yuchenshi
Copy link
Member

yuchenshi commented Mar 14, 2022

Believe it or not, this actually has more to do with your Firestore triggers than the Admin SDK. Thanks for posting the complete logs, and I was able to find this in the logs:

projects/flatz-60aa7/databases/(default)/documents/washingRoutine/{routineId, which is indeed missing a closing }. Please double check the code in your functions source code and add that missing }.

However, we should provide a better error message since now we've seen it twice (after #3136). @taeold Is there anyway we can validate this in the Functions SDK before passing it on to the Firestore Emulator? And/or, the Firestore Emulator should validate this at trigger creation time (before subsequent data requests), which will also make it less misleading.

@taeold
Copy link
Contributor

taeold commented Mar 14, 2022

@yuchenshi Thanks for finding the root cause - and yes I think validating trigger path is a great feature request, just something we didn't have time to implement yet.

Overall, do you know how the trigger validation might work? Can it be just a fancy regex or is there a better way?

@evengul
Copy link
Author

evengul commented Mar 16, 2022

@yuchenshi Thank you! I wasn't able to find this myself, was looking in completely the wrong place. I do agree the error message could be a bit clearer if possible.

@yuchenshi
Copy link
Member

yuchenshi commented Mar 16, 2022

@taeold The Firestore Emulator uses an internal (non-opensource) library function for validation, but I think the format is straightforward to validate with regex as well. Essentially, we just need to make sure the curly brackets match and cannot nest.

@jeevcat
Copy link

jeevcat commented Jun 24, 2022

We're building a Firebase Extension, and I'm also seeing a similar error but as far as I can tell my Firestore trigger path is valid. I think the issue is related to the extension parameter substitution. COLLECTION is an extension parameter. This works for our extension users in production, but not with the emulator.

Path:

projects/${PROJECT_ID}/databases/(default)/documents/${COLLECTION}/{feedId}/{userId}/{foreignId}

Error from firestore-debug.log:

Invalid pattern. Reason: [52:{COLLECTION}/{feedId}/{userId}/{foreignId}] Unrecognized character(s) at end of path.

@taeold taeold changed the title Firestore in Admin SDK /grpc throws IllegalArgumentException: Invalid pattern Improve error message when invalid Firestore document path is specified in Firestore trigger definitions Jan 18, 2023
@taeold taeold removed their assignment Jan 18, 2023
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