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

Invoice Generations fails #453

Open
dalyathan opened this issue Jun 21, 2024 · 2 comments
Open

Invoice Generations fails #453

dalyathan opened this issue Jun 21, 2024 · 2 comments

Comments

@dalyathan
Copy link
Member

dalyathan commented Jun 21, 2024

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

  • NodeJS version: 21
  • Vendure version: 2.1.2
  • Plugin version 2.3.2
  • Database sqljs, mysql
  • Any other version numbers from the environment you're using

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

@dalyathan
Copy link
Member Author

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
Copy link
Member

martijnvdbrug commented Jun 23, 2024

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 ⬆️ 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants