LiveQueryEvent Error Logging Improvements #6951
Conversation
|
Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page. Generated by 🚫 dangerJS |
Codecov Report
@@ Coverage Diff @@
## master #6951 +/- ##
==========================================
+ Coverage 93.78% 93.80% +0.02%
==========================================
Files 169 169
Lines 12252 12260 +8
==========================================
+ Hits 11491 11501 +10
+ Misses 761 759 -2
Continue to review full report at Codecov.
|
| }; | ||
| return maybeRunAfterEventTrigger('afterEvent', className, res); | ||
| }) | ||
| .then(() => { | ||
| if (!res.sendEvent) { |
There was a problem hiding this comment.
That is in relation to preventing events from firing, which was previously achieved by throwing in the trigger. Setting request.sendEvent = false in LQ trigger returns before the events are pushed to the LQ.
There was a problem hiding this comment.
Since maybeRunAfterEventTrigger is the only code that can throw an error this block will be skipped.
There was a problem hiding this comment.
Sorry @dplewis, I'm not sure what the issue is, what would you like me to change? 😊
There was a problem hiding this comment.
Throwing an error handles events from firing. sendEvent can be removed.
There was a problem hiding this comment.
Ok, no worries. I added sendEvent because if you are using a highly restrictive LQ, you'll end up with afterEvent events filling up your logs, which I'm not sure is desired.
For example, only passing LQ update on "foo" change:
Parse.Cloud.afterLiveQueryEvent('TestObject', req => {
if (req.object.get('foo') != req.original.get('foo')) {
// if TestObject has a lot of fields, this will throw often
throw 'Foo was not updated';
}
});
Would log on every single LQ event for TestObject, and pass a subscription.on('error') event. The idea behind the sendEvent is a way to stop events firing, without it sending errors to the client, and without it filling up logLevel: 'error' logs. However happy to remove it if you see it unnecessary.
|
Thanks gents! |
Minor changes to #6859 that I've realised should've been added.
-Throwing in an afterLiveQueryEvent trigger logs properly, instead of "Matching ACL error".
-New sendEvent parameter to afterLiveQueryEvent request payload.
I've been thinking about how the afterLiveQueryEvent should handle not firing events. In the specs, I previously did it by throwing an error in the afterLiveQueryEvent, but I don't think that makes much sense, and also will fill up the logs. Hence, I've added a property "sendEvent" to the afterLiveQueryEvent request payload. If the afterLiveQueryEvent trigger sets that to false, the LiveQuery won't push to the client.
That's all the changes in this PR. I was also thinking about what happens when afterLiveQueryEvent throws an error. Should this be sent to the client as a "error" event, or should it just log on the server (as current behaviour), and no LQ event triggered?
Thank you!