Pinelab-studio / pinelab-vendure-plugins

Monorepo for different Vendure plugins developed by Pinelab
96 stars 42 forks source link

Invoice Generations fails #453

Open dalyathan opened 1 week ago

dalyathan commented 1 week ago

Which plugin/repository is the issue about? @pinelab/vendure-plugin-invoices

Describe the bug When concurrent invoice generation takes place, invoice generations fails. To Reproduce Steps to reproduce the behavior:

  1. Create a Draft Order
  2. Complete at the draft order and add payment to it.
  3. Since the plugin starts generating an invoice when an order is placed, immediately try to create a concurrent invoice generation by clicking Regenerate Invoice button in the order-detail button.
  4. Notice that this fails by throwing the attached error.

Expected behavior invoices should have been regenerated successfully. Screenshots

screenshot

Environment

Additional context The invoice number generation needs to either be transactional scoped or we should use sequences to generate the invoice number

dalyathan commented 1 week ago

The problem occurs because the query in this function doesn't lock the invoice table when it executes. But rather than using locks in entire tables, I think it is better to use sequences to generate the invoiceNumber

martijnvdbrug commented 1 week ago

Hmm good catch. I thought we fixed this one. Can we somehow leverage typeorm to do the auto increment? The difficulty here I think is that the unique constraint should be a combination of invoiceNumber and channelId, because it's fine if different channels have the same invoice numbers.

An alternative is to rewrite the code

  1. Get the latest invoice number
  2. Immediately insert a row with the incremented number, before doing anything else
  3. If a unique constraint fails, we try again (max 3 retries?), until we have a row inserted in the DB. This way we are sure that we have 'reserved' an invoice number.
  4. After that, we generate the PDF and save the storage reference.

The danger here :arrow_up: is that the PDF generation can fail, and we have reserved an invoice number for a PDF that was never created.

Edit: After thinking about it, locking the table during the entire PDF generation process seems like a safe solution. PDF's are generated in the worker, so a bit of a delay during concurrent generations isn't really a problem

Suggestions very welcome!