[16.0][FIX] edi_storage_oca: do not silently ignore OSError/FileNotFoundError

[nilshamerlinck]

Depends on OCA/queue#674

• if an OSError is raised, there can be different cases but it's likely a transient issue (e.g. connection error):

  • if we ignore it as currently:
    • Job won't fail
    • the exchange record will be incorrectly marked as "input_received" but with no data, which is not what we want
  • so we don't ignore it anymore:
    • Job will fail;
    • the record state will stay as "input_pending" (as exception won't be catched in edi.backend:exchange_receive)
    • and as such, another action_exchange_receive Job will be created next time scheduled action "EDI exchange check input sync" runs, to try again to receive it, hopefully successfully this time
    • a RetryableJobError will be raised in order to retry it
    • if it sill can't succeed after max_retries, record state will be changed to "input_receive_error"

• if a FileNotFoundError is raised, it's not a transient issue, so record should be marked as "input_receive_error":

  • to do so we don't ignore it anymore:
    • it will get catched in edi.backend:exchange_receive as a "swallable" (Final) error
    • record state will be changed to "input_receive_error"

[OCA-git-bot]

Hi @etobella, @simahawk,
some modules you are maintaining are being modified, check this out!

[JordiMForgeFlow]

@nilshamerlinck is there a reason why you don't keep the logging? Besides that, the changes look good to me and solve #70

[nilshamerlinck]

good catch @JordiMForgeFlow I can definitely keep it for the raise_if_not_found=False case, fixed!

[simahawk]

LG. Could you please take the chance to add a test for this? 🙏
Also, one consideration:

another action_exchange_receive Job will be created next time scheduled action "EDI exchange check input sync" runs, to try again to receive it, hopefully successfully this time

Wouldn't be better to raise a RetryableJobError then, instead of getting another job?

Of course, if we go this way we should go ahead w/ the fix on queue_job we discussed together regarding matching existing jobs when on error+retry counter>0 😉

[simahawk]

@nilshamerlinck ping :)

[nilshamerlinck]

Hello @simahawk

• @QuocDuong1306 just pushed commits that will apply the same retry logic for receive+process as for send: rely on the RetryableJobError mechanism instead of the scheduled action ("EDI exchange check input sync"), as per your suggestion
• regarding the fix on queue_job, we went for a different approach:
  • instead of preventing the creation of a new job if a failed job with max_retries already exists
  • we explicitly mark the exchange record as KO through a hook, see [16.0][IMP] queue_job: run specific hook method after max_retries queue#674
• thoughts welcome!