GNOME Bugzilla – Bug 764078
Improve error messages
Last modified: 2016-05-26 11:20:32 UTC
Loading the grl-guardian-eyewitness source throws: (grilo-test-ui-0.3:30905): Grilo-WARNING **: [lua-library-operations] grl-lua-library-operations.c:671: operation-id: 1 is on FINALIZED state and cannot be changed (grilo-test-ui-0.3:30905): Grilo-WARNING **: [lua-library] grl-lua-library.c:532: calling source callback function fail: /home/hadess/Projects/gnome-install/share/grilo-plugins/grl-lua-factory/grl-guardian-eyewitness.lua:90: Source is broken as callback was called after the operation has been finalized (grilo-test-ui-0.3:30905): Grilo-WARNING **: [lua-library-operations] grl-lua-library-operations.c:671: operation-id: 2 is on FINALIZED state and cannot be changed (grilo-test-ui-0.3:30905): Grilo-WARNING **: [lua-library] grl-lua-library.c:532: calling source callback function fail: /home/hadess/Projects/gnome-install/share/grilo-plugins/grl-lua-factory/grl-guardian-eyewitness.lua:90: Source is broken as callback was called after the operation has been finalized There are probably more errors than really necessary, and it doesn't explain how to fix it. Did it call grl.callback when it already received all the necessary items? I think that this error is incorrect though.
Yes, it is too verbose, sorry about that. I would prefer to keep the verbosity of the warnings/debug for a little in order to see a problem easily. Regarding the error, it seems correct. You can grl.debug(count) to see that it calls grl.callback() several times till count goes to 0 but after that it calls again with count == -1. Following patch fixes it.
Created attachment 324634 [details] [review] quick fix
Review of attachment 324634 [details] [review]: OK. That means it's likely a bug in the backend that returns items than requested. Still, the error message should mention (as it used to) that the source has already been called with "0" items left, rather than it being in a finalized state. I'll investigate the eyewitness bug itself now.
(In reply to Bastien Nocera from comment #3) > Review of attachment 324634 [details] [review] [review]: > > OK. That means it's likely a bug in the backend that returns items than > requested. > > Still, the error message should mention (as it used to) that the source has > already been called with "0" items left, rather than it being in a finalized > state. Sure, I'll try to improve the messages soon.
FWIW, the handling of count/skip/page was completely wrong in the source, I fixed that in grilo-lua-sources. Leaving this open for the better error message.
This is still a problem. When there's a syntax error, I would get something like: (test_games:30940): Grilo-WARNING **: [lua-library-operations] grl-lua-library-operations.c:538: Source 'grl-thegamesdb' is broken, as the finishing callback was not called for resolve operation Instead of an error about the syntax error.
Created attachment 328231 [details] [review] lua-factory: include error message for broken source On grl_lua_operations_get_current_op failure, the lua-library api now issues a lua_error which points to file and line number where the error happened. If we can't retrieve the current operation it means that grl.callback() was called and the grilo operation has been finalized.
Created attachment 328232 [details] [review] lua-factory: fix syntax-check for lua-sources
Created attachment 328233 [details] [review] metrolyrics: lower failure from warning to debug
Review of attachment 328231 [details] [review]: > On grl_lua_operations_get_current_op failure, the lua-library api now When grl_lua_operations_get_current_op() fails... issues a lua_error which points to file and line number where the error happened. If we can't retrieve the current operation it means that grl.callback() was called and the grilo operation has been finalized. ::: src/lua-factory/grl-lua-library-operations.c @@ +668,3 @@ * watchdog to free its data. Only a broken source would request * OperationSpec on FINALIZED State */ + GRL_DEBUG ("The grilo operation has ended on grl.callback call. " The grilo operation ended when grl.callback() was called. ::: src/lua-factory/grl-lua-library.c @@ +747,3 @@ + if (os == NULL) { + luaL_error (L, "grl.get_options failed: Can't retrieve current operation. " + "Source is broken as grl.callback has been called but source " Line up that line with the quotes on the line above. And everywhere else, I'd rather have grl.callback() than a bare one. @@ -1445,3 @@ GRL_DEBUG ("grl.unzip() -> '%s'", url); - wc = net_wc_new_with_options (L, 3); That's a separate fix, isn't it?
Review of attachment 328232 [details] [review]: Looks good.
Review of attachment 328233 [details] [review]: Yes.
Created attachment 328301 [details] [review] lua-factory: include error message for broken source When grl_lua_operations_get_current_op() fails issues a lua_error which points to file and line number where the error happened. If we can't retrieve the current operation it means that grl.callback() was called and the grilo operation has been finalized.
Created attachment 328302 [details] [review] lua-factory: avoid leak of GrlNetWc on failure
Created attachment 328529 [details] [review] lua-factory: support to cancel operation Grilo has optional method to cancel operations which Lua-Factory had not implemented so far, till now. For each operation we create a GCancellabe that should be used in async functions in Lua-Library such as grl.fetch and grl.unzip. The grl_lua_operations_cancel_operation(), will cancel the operation, remove the OperationSpec from its internals and free the related data. https://bugzilla.gnome.org/show_bug.cgi?id=764077
Review of attachment 328529 [details] [review]: wrong bug.
Review of attachment 328301 [details] [review]: Looks good.
Review of attachment 328302 [details] [review]: Sure.
Attachment 328232 [details] pushed as 3c0e9a0 - lua-factory: fix syntax-check for lua-sources Attachment 328233 [details] pushed as 51a0e32 - metrolyrics: lower failure from warning to debug Attachment 328301 [details] pushed as 8f66902 - lua-factory: include error message for broken source Attachment 328302 [details] pushed as b53c902 - lua-factory: avoid leak of GrlNetWc on failure