- Notify engine to not observe image preview any more on error codition
- Shutdown image loader, HTTP job simultaneously on error condition
Details
- Reviewers
leinir dfaure - Commits
- R304:ab6a48778bbe: [knewstuff] Do not leak ImageLoader on error
We shouldn't restart image loader on error condition, no ?
Diff Detail
- Repository
- R304 KNewStuff
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
src/core/engine.cpp | ||
---|---|---|
536 | A more descriptive error message would be good, but this is already an improvement over no error message at all :) As pointed out elsewhere, since you're explicitly passing the ImageLoader instance through the signal, might it not make sense to output the description of the error (which is available on the httpjob, which in turn is available through ImageHandler::job, so you could just add l to the capture to access that)? |
src/core/engine.cpp | ||
---|---|---|
536 | You did add l to the capture, but unless i'm missing something super obvious... you're not using it ;) i meant to do something like add << l->job()->errorText() to the debug statement so we can see what the job says has gone wrong (which arguably is the more useful information here)... Maybe also emit a signalError with an appropriate text (because that's how to tell the user about errors)... though i'm not entirely certain if we might not want to just fail quietly here. |
src/core/engine.cpp | ||
---|---|---|
536 | Job can be nullptr, so signal can be a text, it can capture entry by reference? |
src/core/engine.cpp | ||
---|---|---|
536 | i guess you could extend the signal further with the error text added in instead of capturing l in the lambda, yes |