Merge lp:~camptocamp/openobject-addons/trunk-fix1003819 into lp:openobject-addons/6.1

[Alexandre Fayolle - camptocamp]

fix lp:1003819

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

We must merge your changeset :-)

review: Approve

[lp:~camptocamp/openobject-addons/trunk-fix1003819]

6915. By Alexandre Fayolle @ camptocamp <email address hidden> on 2012-07-24

[FIX] report_webkit: ensure the temp file fd is always closed

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

I added related merge requests on the trunk/7.0 branch and on the 6.0 branch

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

LGTM

review: Approve (code review only)

Unmerged revisions

6915. By Alexandre Fayolle @ camptocamp <email address hidden> on 2012-07-24

[FIX] report_webkit: ensure the temp file fd is always closed

6914. By Alexandre Fayolle @ camptocamp <email address hidden> on 2012-07-24

[FIX] fix issue lp:1003819

* process the information that wkhtmltopdf can write on stderr
* cleanup the code handling file deletion to remove duplication
* provide information to the user about what went wrong

=== modified file 'report_webkit/webkit_report.py'
--- report_webkit/webkit_report.py	2012-02-16 10:58:58 +0000
+++ report_webkit/webkit_report.py	2012-07-24 13:36:21 +0000
@@ -107,7 +107,7 @@
         tmp_dir = tempfile.gettempdir()
         out_filename = tempfile.mktemp(suffix=".pdf", prefix="webkit.tmp.")
         files = []
-        file_to_del = []
+        file_to_del = [out_filename]
         if comm_path:
             command = [comm_path]
         else:
@@ -160,22 +160,33 @@
             file_to_del.append(html_file.name)
             command.append(html_file.name)
         command.append(out_filename)
+        stderr_fd, stderr_path = tempfile.mkstemp(text=True)
+        file_to_del.append(stderr_path)
         try:
-            status = subprocess.call(command, stderr=subprocess.PIPE) # ignore stderr
+            status = subprocess.call(command, stderr=stderr_fd)
+            os.close(stderr_fd) # ensure flush before reading
+            stderr_fd = None # avoid closing again in finally block
+            fobj = open(stderr_path, 'r')
+            error_message = fobj.read()
+            fobj.close()
+            if not error_message:
+                error_message = _('No diagnosis message was provided')
+            else:
+                error_message = _('The following diagnosis message was provided:\n') + error_message
             if status :
-                raise except_osv(
-                                _('Webkit raise an error' ),
-                                status
-                            )
-        except Exception:
-            for f_to_del in file_to_del :
-                os.unlink(f_to_del)
-
-        pdf = file(out_filename, 'rb').read()
-        for f_to_del in file_to_del :
-            os.unlink(f_to_del)
-
-        os.unlink(out_filename)
+                raise except_osv(_('Webkit error' ),
+                                 _("The command 'wkhtmltopdf' failed with error code = %s. Message: %s") % (status, error_message))
+            pdf_file = open(out_filename, 'rb')
+            pdf = pdf_file.read()
+            pdf_file.close()
+        finally:
+            if stderr_fd is not None:
+                os.close(stderr_fd)
+            for f_to_del in file_to_del:
+                try:
+                    os.unlink(f_to_del)
+                except (OSError, IOError), exc:
+                    logger.error('cannot remove file %s: %s', f_to_del, exc)
         return pdf
 
     def translate_call(self, src):