We should raise an error when wkhtmltopdf fails

Bug #1003563 reported by Alexis de Lattre
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Camptocamp Financial Addons
Fix Released
Medium
Alexandre Fayolle - camptocamp

Bug Description

Dear Camptocamp dream team,

In c2c-financial-addons/account_financial_report_webkit/report/webkit_parser_header_fix.py, there is the following code arround line 119 :

        try:
            status = subprocess.call(command, stderr=subprocess.PIPE) # ignore stderr
            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)

The problem is the following :
if the wkhtmltopdf command fails, then status <> 0 and you raise an exception ; this exception is catched and you remove the HTML file... and then the program goes on ! No error message is displayed to the user !

In my case, wkhtmltopdf failed with the error "wkhtmltopdf: cannot connect to X server"... and it eventually crashes later on in the code and I get the Publisher warranty error pop-up with message :

OpenERP-Client Version : 6.1.1
Last revision No. & ID :2059 <email address hidden>
<ProtocolError for localhost:9998/xmlrpc/report: 500 INTERNAL SERVER ERROR>

and on the server side, the message was :

2012-05-23 17:10:33,208 20396 ERROR ? openerp.service.web_services: Exception: [Errno 2] No such file or directory: '/tmp/webkit.tmp.McE6mh.pdf'
Traceback (most recent call last):
  File "/opt/openerp/prod/server/openerp/service/web_services.py", line 727, in go
    (result, format) = obj.create(cr, uid, ids, datas, context)
  File "/opt/openerp/prod/addons/report_webkit/webkit_report.py", line 325, in create
    result = self.create_source_pdf(cursor, uid, ids, data, report_xml, context)
  File "/opt/openerp/prod/server/openerp/report/report_sxw.py", line 511, in create_source_pdf
    return self.create_single_pdf(cr, uid, ids, data, report_xml, context)
  File "/opt/openerp/prod/c2c-financial-addons-6.1/account_financial_report_webkit/report/webkit_parser_header_fix.py", line 223, in create_single_pdf
    pdf = self.generate_pdf(bin, report_xml, head, foot, htmls)
  File "/opt/openerp/prod/c2c-financial-addons-6.1/account_financial_report_webkit/report/webkit_parser_header_fix.py", line 130, in generate_pdf
    pdf = file(out_filename, 'rb').read()
IOError: [Errno 2] No such file or directory: '/tmp/webkit.tmp.McE6mh.pdf'
2012-05-23 17:10:33,966 20396 ERROR ? openerp.netsvc: 2
No such file or directory
(<type 'exceptions.IOError'>, IOError(2, 'No such file or directory'), <traceback object at 0xb194ae3c>)
2012-05-23 17:10:33,983 20396 INFO ? werkzeug: 127.0.0.1 - - [23/May/2012 17:10:33] "POST /xmlrpc/report HTTP/1.1" 500 -
2012-05-23 17:10:33,993 20396 ERROR ? werkzeug: Error on request:
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/werkzeug/serving.py", line 159, in run_wsgi
    execute(app)
  File "/usr/local/lib/python2.7/dist-packages/werkzeug/serving.py", line 146, in execute
    application_iter = app(environ, start_response)
  File "/opt/openerp/prod/server/openerp/wsgi/core.py", line 397, in application
    result = handler(environ, start_response)
  File "/opt/openerp/prod/server/openerp/wsgi/core.py", line 222, in wsgi_xmlrpc_legacy
    return xmlrpc_return(start_response, path, method, params, True)
  File "/opt/openerp/prod/server/openerp/wsgi/core.py", line 83, in xmlrpc_return
    response = xmlrpc_handle_exception_legacy(e)
  File "/opt/openerp/prod/server/openerp/wsgi/core.py", line 124, in xmlrpc_handle_exception_legacy
    fault = xmlrpclib.Fault('warning -- ' + e.name + '\n\n' + e.value, '')
TypeError: coercing to Unicode: need string or buffer, tuple found

The enclosed patch will block the execution if wkhtmltopdf fails and will display the error code to the user in a nice pop-up.

It would be even better to display the output of stderr, so that it's easier for the user to understand the cause of the issue. With my enclosed patch, the pop-up would say "The command 'wkhtmltopdf' failed with error code = 1", which won't help me solve the issue. If I get the message "The command 'wkhtmltopdf' failed with the following message : wkhtmltopdf: cannot connect to X server", then it would be easier for me to solve it. But I am not an expert of subprocess.call and I don't know how it can be done.

The enclosed patch has been successfully tested on the branch lp:c2c-financial-addons/6.1/

P.S. : for those we have the same error with wkhtmltopdf, here is the explaination : http://code.google.com/p/wkhtmltopdf/issues/detail?id=3

Related branches

Revision history for this message
Alexis de Lattre (alexis-via) wrote :
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote : Re: [Bug 1003563] Re: We should raise an error when wkhtmltopdf fails

On mer. 23 mai 2012 19:28:33 CEST, Alexis de Lattre wrote:
> ** Patch added: "patch-c2c-financial-addons-6.1_webkit-error.diff"
> https://bugs.launchpad.net/bugs/1003563/+attachment/3159753/+files/patch-c2c-financial-addons-6.1_webkit-error.diff
>

Thanks for the bug report Alexis.

I'm going to look into this, and propose an updated version of the
patch to include the output of the failing command. Stay tuned.

--
Alexandre Fayolle
Chef de Projet
Tel : + 33 (0)4 79 26 57 92

Camptocamp France SAS
Savoie Technolac, BP 352
73377 Le Bourget du Lac Cedex
http://www.camptocamp.com

Changed in c2c-financial-addons:
assignee: nobody → Alexandre Fayolle @ camptocamp (alexandre-fayolle-c2c)
status: New → Confirmed
importance: Undecided → Medium
Changed in c2c-financial-addons:
status: Confirmed → In Progress
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

Alexis,

I've just submitted a fix for this bug as a merge request. Could you take the time to check that it does fix your issue (I don't have an instance at hand with no X server which I can use to generate a systematic error) and report as a review of https://code.launchpad.net/~c2c/c2c-financial-addons/6.1-fix-1003563/+merge/107169 ?

Thanks in advance

--
Alexandre

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

it looks like I goofed my merge request. Will try again. Sorry for the inconvenience.

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

looks better now.

Revision history for this message
Alexis de Lattre (alexis-via) wrote :

OK, I added my comment on the merge proposal (according to my tests, the issue is still present).

Changed in c2c-financial-addons:
status: In Progress → Fix Committed
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Can we put this bug to "Fix Released" ?

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

done

Changed in c2c-financial-addons:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.