Conversation
schiwekM
left a comment
There was a problem hiding this comment.
Thanks again for the changes. We will add docs to the readme to document the events and then the PR is good to go.
Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
I just had another point, when I saw your review: would it be beneficial to emit those events inside |
|
That should be the case ootb. Emit should write the events to the outbox, the transaction continues. And the outbox picks up the event, starts a new transaction and calls the handler |
Just for clarification - in the docs for the outbox (see https://pages.github.tools.sap/cap/docs/node.js/queue#persistent-queue):
If I understand this correctly, the event won't be written to the outbox but the request's transaction is rolled back via |
|
Ahh, so you want to even emit the event in cases where the transaction is rolled back? Yes in that case we need to wrap it inside cds.spawn |
Yes, if you look in the code, the request gets rejected if for example the attachment is too large. I will adapt it accordingly. |
…ct even if transaction is rolled back
…ithub.com:ansgarlichter/attachments into feat/emit-events-for-security-relevant-rejections
Closes #400.
If you do not see the feature request as a valid enhancement, just close the PR.