- Merge request !115Issue #3158678: FetcherResult::checkFile() throws \RuntimeException, which is never caught → (Open) created by megachriz
- 🇳🇱Netherlands megachriz
It has been a while since I last worked on this issue, but in the branch 3158678-tests_only I had made a draft for a test. I couldn't post it before because it depended on the changes from 📌 Missing temporary files in load balanced environments Fixed . I'm posting it now and plan to look into it in more detail later.
- 🇳🇱Netherlands megachriz
I took a brief look at this issue today. I think something like to proposed fix looks okay, but I think we should use
watchdog_exception()
and the equivalent of that → in Drupal 10.I do wonder the following:
- Should other types of exceptions be catched too? Maybe not. The thing is it could be bad if any exception aborts the import process. But it is bad either if the same exception occurs again and again, because then the import never continues.
- Is there a case where a RuntimeException is temporary? Because if there is, then it might be bad that the import just gets aborted?
- 🇳🇱Netherlands megachriz
I think that 🐛 in_progress filesystem grows indefinitely Active is closely related. To try to move this issue forward, here's what I plan to do:
- Add the test from 🐛 in_progress filesystem grows indefinitely Active to this one.
- Implement a fix almost the same as what is in #9, with the difference of using
watchdog_exception()
(+ the equivalent of that → for Drupal 10) - See if that makes the test pass.
There is however a risk of introducing a regression with the proposed fix: some imports that now go through after a temporary error may then instead get aborted. But maybe we should see if any issue reports come up after making a change?
It is about the following type of situation: an import is running on cron, but during the import a resource becomes unavailable due to a hitch.
- In the current situation, the import gets halted. During the next cron run, Feeds retries the import. If the hitch is then over, then import will continue.
- In the new situation, a hitch can abort the import process (if the type of exception is "RuntimeException"). The import would then have to be restarted from zero by the user (or it gets restarted automatically after some time if periodic import is configured).
However, it looks like the situation where an error happens that is not temporary is way more likely to happen. In this case, Feeds keeps trying over and over again and that makes the import to never finish. In case of 🐛 in_progress filesystem grows indefinitely Active it is even worse: a file to import gets downloaded and on the next try it gets downloaded again. And again. Resulting into a huge pile of files over time.
Thoughts on this issue are welcome! Hopefully I can get the steps outlined here done soon.
- last update
over 1 year ago 691 pass, 3 fail - Merge request !127Issue #3158678: FetcherResult::checkFile() throws \RuntimeException, which is never caught → (Merged) created by megachriz
- last update
over 1 year ago 696 pass, 2 fail - last update
over 1 year ago 703 pass - last update
over 1 year ago 691 pass, 3 fail - Status changed to Needs review
over 1 year ago 9:59am 30 July 2023 - last update
over 1 year ago Patch Failed to Apply - 🇭🇺Hungary aron novak Hungary, Budapest
MegaChriz → credited Aron Novak → .
- last update
over 1 year ago 704 pass 38:46 38:02 Running- 🇨🇭Switzerland x775
Hi @MegaChriz
We are still seeing errors similar to
`RuntimeException: File /tmp/feeds_http_fetcherhNFdBE does not exist. in Drupal\feeds\Result\FetcherResult->checkFile() (line 53 of feeds/src/Result/FetcherResult.php).`
despite running 8.x-3.0-beta4.
Do we need to update?
Thanks!
- 🇳🇱Netherlands megachriz
@x775
It's possible that some items are still left in the queue. The changes in 8.x-3.0-beta4 only aimed to fix problems like you are having for new imports. However, the code changes on this issue should fix it also for "left over" tasks. Did you try the code from this issue? 13:21 10:16 Running11:30 8:14 Running- 🇳🇱Netherlands megachriz
Now that 🐛 in_progress filesystem grows indefinitely Active and 🐛 Delete Empty Files Created If Empty/Error Exception is Returned Needs review are done, I could perhaps give this issue another look.
- last update
8 months ago 713 pass, 1 fail - last update
8 months ago 713 pass, 2 fail - last update
8 months ago 715 pass, 1 fail - last update
8 months ago 722 pass - 🇳🇱Netherlands megachriz
I reduced the scope of the issue this is fixing. This reduces the chance of regressions, which was I afraid for, but it’s also possible that doesn’t fix the reported issue entirely.
Now an import gets aborted only when the file to import (which is saved in the "in_progress" directory) no longer exists. When Feeds fails to fetch a resource, for example because of a 404, Feeds will not abort the import process on cron runs. Instead, Feeds will retry the fetch on the next cron. It does so forever, which is an issue as well, but I think we should address that in 🐛 Feed fetch errors don't seem to be handled gracefully Active instead.
By doing so, we can at least get a part of the issue fixed and at the same time reduce the chance of regressions. - last update
8 months ago 722 pass - last update
8 months ago 722 pass -
MegaChriz →
committed b6787225 on 8.x-3.x
Issue #3158678 by MegaChriz, bburg, Steven Jones, Aron Novak: Catch...
-
MegaChriz →
committed b6787225 on 8.x-3.x
- Status changed to Fixed
8 months ago 7:44am 8 June 2024 Automatically closed - issue fixed for 2 weeks with no activity.