Firestore OnWrite Trigger Logic Bug on Document Change
Firestore OnWrite Trigger Logic Bug on Document Change
Background:
We've built an application that works like a forum:
Problem:
The current implementation has a bug whereby anytime an answer is edited, the counter for that thread it increased. The IF STATEMENT that causes the problem seems to be if (!change.before.exists).
if (!change.before.exists)
Obviously, we've tried debugging this ourselves, but have failed to find an appropriate solution. Any help that you could provide is appreciated.
Code:
exports = module.exports = functions.firestore
.document('questions/questionId/answers/answerId')
.onWrite(async (change, context) =>
var question = await FireStoreUtils.getDocumentSimple('questions', context.params['questionId'])
if (!question)
throw new functions.https.HttpsError('invalid-argument', 'Question does not exist')
var payload =
/*
* This if statement detects when a new answer has
* been posted
*/
if (!change.before.exists)
payload['answers'] = question.data().answers + 1
/*
* This if statement detects when a new answer has
* been deleted
*/
if (!change.after.exists)
payload['answers'] = question.data().answers - 1
if (payload['answers'])
return firebase.firestore()
.collection('questions')
.doc(context.params['questionId'])
.update(payload)
.then(() =>
return 'Success'
)
else
return true
)
Expected Outcome:
The reason why we've selected onWrite is because the code base is already large, and we find that there's a lot of code repeat against multiple Cloud Functions. Thus, creating two functions, in the long run, is likely to make things more complicated than they need to be. Technically speaking, we could do it, although, our preference would be to try and find a way to resolve this bug in our current implementation first.
– ABC
Aug 28 at 1:19
Your function is not really that long at all, and you will reduce its size by splitting it into two functions anyway, because you will not have to check the change object.
– Doug Stevenson
Aug 28 at 1:31
Also, you're not supposed to throw
functions.https.HttpsError from functions that are not callable functions. In fact, you shouldn't throw an exception at all in this case, especially if you ever want to enable retries on it (because it would retry forever).– Doug Stevenson
Aug 28 at 1:32
functions.https.HttpsError
Doug, thank you for your input. Clearly, you don't have the answer on how to fix this particular bug. If in the next 24 hours, nobody can spot what the issue is, then we'll have no other choices than to split this function into two it seems.
– ABC
Aug 28 at 4:28
By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.
Why don't you just use onCreate and onDelete to modify the counter? That way you don't have to write code to check what happened with the write.
– Doug Stevenson
Aug 28 at 0:32