pablrod / p5-Chart-Plotly

Generate html/javascript charts with perl data using javascript library Plotly.js
MIT License
9 stars 4 forks source link

save_image - Permission denied error while calling renaming generated image in Chart::Kaleido::Plotly #58

Open rai-gaurav opened 3 years ago

rai-gaurav commented 3 years ago

Chart-Plotly version - 0.041 OS - Windows 10 Perl - Strawberry 5.30.1

Just using the example mentioned here

save_image(file => 'TestOrca.png', plot => $plot,
           width => 1024, height => 768,
           engine => 'auto');

This call to save_image is generating warning while running the script.

Error rename on 'D:/ChartPlotly/output/lineChart.png19988698538865' -> 'D:/ChartPlotly/output/lineChart.png': Permission denied at C:/berrybrew/5.30.1_64/perl/site/lib/Chart/Kaleido/Plotly.pm line 160.
Warning: unable to close filehandle GEN10 properly: Bad file descriptor during global destruction.
Warning: unable to close filehandle GEN5 properly: Bad file descriptor during global destruction.

The file is generated perfectly only facing error while renaming it. The error is generated in Chart::Kaleido::Plotly at

path($file)->spew_raw($img);

I tried to dig more and found the documentation for 'spew_raw' here. There is a 'Note' section which mentions -

because the file is written to a temporary file and then renamed, the new file will wind up with permissions based on your current umask.
This is a feature to protect you from a race condition that would otherwise give different permissions than you might expect. If you really want to keep the original mode flags, use "append" with the truncate option.

I have just tried to change the error generating line in Chart::Kaleido::Plotly to -

path($file)->append_raw({truncate => 1}, $img);

and I am not getting any rename error and the final image file is generating fine. However, the other two lines of warning are still there

Warning: unable to close filehandle GEN10 properly: Bad file descriptor during global destruction.
Warning: unable to close filehandle GEN5 properly: Bad file descriptor during global destruction.

I have tested it on Windows, so not sure what will be the effect of this change on Linux and others. Let me know if you want more details.

pablrod commented 3 years ago

Thank you for this detailed report!

I don't have access anymore to a windows machine so if you don't mind to test some things I'll be glad to help.

Please, try this example: (extracted from: https://metacpan.org/pod/Chart::Kaleido::Plotly#SYNOPSIS)

use Chart::Kaleido::Plotly;
use JSON;

my $kaleido = Chart::Kaleido::Plotly->new();

# convert a hashref
my $data = decode_json(<<'END_OF_TEXT');
{ "data": [{"y": [1,2,1]}] }
END_OF_TEXT
$kaleido->save( file => "foo.png", plot => $data,
                width => 1024, height => 768 );
rai-gaurav commented 3 years ago

I got the similar error with this example.

Error rename on 'foo.png21276618096281' -> 'foo.png': Permission denied at C:/berrybrew/5.30.1_64/perl/site/lib/Chart/Kaleido/Plotly.pm line 160.
Warning: unable to close filehandle GEN10 properly: Bad file descriptor during global destruction.
Warning: unable to close filehandle GEN5 properly: Bad file descriptor during global destruction.
pablrod commented 3 years ago

Ok!

It seems a bug in Chart::Kaleido::Plotly. Please @stphnlyd could you look at this?

Using the other engine for images: Orca should work fine:

save_image(file => 'TestOrca.png', plot => $plot,
           width => 1024, height => 768,
           engine => 'orca');

But it requires the installation of Orca. With

cpanm Alien::Plotly::Orca
iynehz commented 3 years ago

@pablrod Sorry only saw this now. I will take a look maybe later this week. Sigh: Windows is hard for Perl. Actually I have another issue in Chart::Kaleido::Plotly with IPC::Run https://github.com/stphnlyd/perl5-Chart-Kaleido/issues/1 If we can't easily solve this in the end, maybe we should use Orca by default on Windows.

pablrod commented 3 years ago

Orca could be a sensible default on Windows. Just let me know and I'll make the change!

Thank you!

iynehz commented 3 years ago

I think @rai-gaurav 's idea of s/spew_raw/append_raw/ makes sense. I've released a new version of Chart-Kaleido to CPAN with that change. Will see if I can look IPC::Run later this week..