SpartnerNL / Laravel-Excel

🚀 Supercharged Excel exports and imports in Laravel
https://laravel-excel.com
MIT License
12.28k stars 1.92k forks source link

[Bug]: Creating exports on Laravel Vapor: Could not find zip member zip #4177

Open fransjooste1 opened 2 months ago

fransjooste1 commented 2 months ago

Is the bug applicable and reproducable to the latest version of the package and hasn't it been reported before?

What version of Laravel Excel are you using?

3.1.55

What version of Laravel are you using?

11.19.0

What version of PHP are you using?

8.3

Describe your issue

When doing exports within a Vapor environment, it seems like the AppendQueryToSheet job is spawned before the file is fully created on S3.

Keep running into: PhpOffice\PhpSpreadsheet\Reader\Exception Could not find zip member zip:///tmp/laravel-excel-BSReujKfyopUuGIeaQy50PP4DyuxvXee.xlsx#_rels.rels

When viewing S3 the file does exist. Additionally, sometimes the export works, but most of the times it fails. Vapor (AWS) doesn't allow you to create a queue of size 1, so the other chunks of jobs are usually picked up by another worker.

    (new EventFinancialsExport($this->event))->queue($path, 'exports')->allOnQueue('queue-exports')->chain(
        [
            new SendExportCompletedNotification($user, $path),
        ]
    );

Config:

'temporary_files' => [
    'remote_disk' => 'tmp',
];

FileSystem:

'tmp' => [
    'driver' => 's3',
    'key' => env('AWS_ACCESS_KEY_ID'),
    'secret' => env('AWS_SECRET_ACCESS_KEY'),
    'region' => env('AWS_DEFAULT_REGION'),
    'bucket' => env('AWS_BUCKET'),
    'url' => env('AWS_URL'),
    'directory_separator' => '/',
    'root' => 'tmp/',
],

How can the issue be reproduced?

This isn't easy to recreate unfortunately.

You need to be on Laravel Vapor. Create an environment with queued workers. Create an export with a chained mail or reporting job on a specific queue. You need to export items that has a large number of rows, forcing chucking of query results. The jobs being spawned will grab the file before it is ready creating the error.

What should be the expected behaviour?

I think a solution could be just to wrap the AppendQueryToSheet within a quick retry function or anywhere where the file is being accessed if jobs were spawned, just to try and get the file again creating a small delay. I believe another job is fired too fast off the queue and is accessing the file while it is being created on S3 in an incomplete state.

This is how I solved the same issue (error was the same) in another line of code where we would download the file directly if small enough.

// This fixed the same issue as described above
$export = (new EventFinancialsExport($this->event));

return retry(5, function () use ($export, $path) {
    return $export->download($path);
}, 100, function ($exception) {
    $this->emitNotification('An error occurred while generating the report. Please try again or contact support.');
});

I could be entirely off the mark also, any feedback or suggestions would be greatly appreciated.

devinfd commented 2 months ago

I'm not certain why but I just recently started to get the same error. Also using a s3 bucket for the temp_filder

patrickbrouwers commented 2 months ago

I don't use Vapor myself, so I have no clue what exactly goes wrong. Feel free to PR a fix

chatisk commented 2 months ago

I am on vapor and I can confirm this happens too. And it's been quite a problem lately.

devinfd commented 2 months ago

@fransjooste1 @chatisk We need to figure this out. What are the commonalities between our projects besides Vapor? Are you using Octane?

fransjooste1 commented 2 months ago

@devinfd yes we are also using Octane

devinfd commented 2 months ago

Ok, Octane is my first guess to the source of the problem. I suspect this is not a Laravel Excel problem but I'm not certain yet. I'm going to spend some time on this bug over the next few days.

chatisk commented 2 months ago

@devinfd I am not using Octane. We use S3, and we have filled both settings for temporary files. Take a look

'local_path'          =>  env('APP_ENV') === 'local'
            ? storage_path('framework/cache/laravel-excel')
            : sys_get_temp_dir(),
'remote_disk'         => 's3-tenant', // Multi-tenancy,
'force_resync_remote' => true,
jbajou commented 1 month ago

Same project as @chatisk here. We tried with remote_disk => null because of the comment in the config file:

/*
        |--------------------------------------------------------------------------
        | Remote Temporary Disk
        |--------------------------------------------------------------------------
        |
        | When dealing with a multi server setup with queues in which you
        | cannot rely on having a shared local temporary path, you might
        | want to store the temporary file on a shared disk. During the
        | queue executing, we'll retrieve the temporary file from that
        | location instead. When left to null, it will always use
        | the local path. This setting only has effect when using
        | in conjunction with queued imports and exports.
        |
        */

No better luck. The exception we get is: PhpOffice\PhpSpreadsheet\Reader\Exception: File "/tmp/laravel-excel-TTaINcwtkPaXabycDhtWB518DmnvPn9N.xlsx" does not exist.

Our temp storages are configured as follow (vapor.yml):

        queue-tmp-storage: 1024
        tmp-storage: 1024
        cli-tmp-storage: 512

As @fransjooste1 mentioned in the first post, it's all like the system is trying to access the export while it's not there already. It's like the workers are running in parallel, not on the same container (so not the same temp storage).

mxmtsk commented 1 month ago

Same here, also not on Octane but on plain Laravel & Vapor. Our config looks about the same as mentioned above

patrickbrouwers commented 1 month ago

It's possible this PR https://github.com/SpartnerNL/Laravel-Excel/pull/4034 has something to do with it. I don't use Vapor (or a multi server setup) (and no time to set it up to test this), so If someone wants to figure this out, feel free to PR a fix.

jbajou commented 1 month ago

just as a follow up, commenting L169 to L172 of src/Writer.php works for us with Vapor.

//        if ($temporaryFile instanceof RemoteTemporaryFile && !$temporaryFile->existsLocally()) {
//            $temporaryFile = resolve(TemporaryFileFactory::class)
//                ->makeLocal(Arr::last(explode('/', $temporaryFile->getLocalPath())));
//        }
devinfd commented 1 month ago

Hi all, I no longer believe that Octane has anything to do with this bug. I agree/suspect with @jbajou and @fransjooste1 assessment that the file "system is trying to access the export while it's not there already"

jbajou commented 1 month ago

I just PR'ed something that addresses the issue. I've deployed it in our sandbox env and it's working so far.

Are you guys able to test it on your end too ?

patrickbrouwers commented 1 month ago

If people using Vapor can let me know if the PR works, I'll release a fix with it

devinfd commented 1 month ago

The soonest I can try is this weekend. I’ll let you know soon after

keithbrink commented 1 month ago

@jbajou I'm not on a serverless environment and I get the same error. We are running in k8s and have multiple queue workers running. Is a more general check possible?

chatisk commented 1 month ago

@jbajou I'm not on a serverless environment and I get the same error. We are running in k8s and have multiple queue workers running. Is a more general check possible?

The PR has been merged and is released. Our issue was resolved like that. Perhaps you need to add some extra checks on the new function introduced isServerless()

jbajou commented 1 month ago

The PR adresses only runs on AWS as our issues is on lambda functions. For kubernetes, you'd have to find a way to understand how you can see it runs in K8, maybe an env variable again. Did you try commenting the lines altogether to see if it fixes your issue ? (I guess it will as you are probably ending on another container, assuming your temp storage exists, is writable etc) If you do find a way and are up to PR it, maybe the method I created should be renamed to reflect your change.

liepumartins commented 3 weeks ago

Can confirm the same issue. Been struggling with this since export size got bigger than 1 request could handle.

After upgrading 3.1.55 => 3.1.58 it feels that the problem has actually gotten worse - even less chance of successful export.

Serverless Vapor. Without Octane.

I actually tried to work around this using failed() method, but it never gets called. #4078 Now I understand that the issue lies elsewhere.

EDIT: isRunningServerless() correctly skips the

if ($temporaryFile instanceof RemoteTemporaryFile && !$temporaryFile->existsLocally() && !$this->isRunningServerless()) {
$temporaryFile = resolve(TemporaryFileFactory::class)
->makeLocal(Arr::last(explode('/', $temporaryFile->getLocalPath())));
}

But fails on this:

$writer->save(
$temporaryFile->getLocalPath()
);

Relevant stacktrace

ErrorException:
fopen(/tmp/storage/framework/cache/laravel-excel/laravel-excel-I8nSadaIegJiuyoBXrOOjLTwomglGcL2.xlsx): Failed to open stream: No such file or directory

  at /var/task/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/BaseWriter.php:128
  at Illuminate\Foundation\Bootstrap\HandleExceptions->handleError(2, 'fopen(/tmp/storage/framework/cache/laravel-excel/laravel-excel-I8nSadaIegJiuyoBXrOOjLTwomglGcL2.xlsx): Failed to open stream: No such file or directory', '/var/task/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/BaseWriter.php', 128)
     (/var/task/vendor/laravel/framework/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php:255)
  at Illuminate\Foundation\Bootstrap\HandleExceptions->Illuminate\Foundation\Bootstrap\{closure}(2, 'fopen(/tmp/storage/framework/cache/laravel-excel/laravel-excel-I8nSadaIegJiuyoBXrOOjLTwomglGcL2.xlsx): Failed to open stream: No such file or directory', '/var/task/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/BaseWriter.php', 128)
  at fopen('/tmp/storage/framework/cache/laravel-excel/laravel-excel-I8nSadaIegJiuyoBXrOOjLTwomglGcL2.xlsx', 'wb')
     (/var/task/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/BaseWriter.php:128)
  at PhpOffice\PhpSpreadsheet\Writer\BaseWriter->openFileHandle('/tmp/storage/framework/cache/laravel-excel/laravel-excel-I8nSadaIegJiuyoBXrOOjLTwomglGcL2.xlsx')
     (/var/task/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/Xlsx.php:551)
  at PhpOffice\PhpSpreadsheet\Writer\Xlsx->save('/tmp/storage/framework/cache/laravel-excel/laravel-excel-I8nSadaIegJiuyoBXrOOjLTwomglGcL2.xlsx')
     (/var/task/vendor/maatwebsite/excel/src/Writer.php:184)
  at Maatwebsite\Excel\Writer->write(object(OrderItemExport), object(RemoteTemporaryFile), 'Xlsx')
     (/var/task/vendor/maatwebsite/excel/src/Jobs/QueueExport.php:81)
  at Maatwebsite\Excel\Jobs\QueueExport->Maatwebsite\Excel\Jobs\{closure}(object(QueueExport))
     (/var/task/vendor/maatwebsite/excel/src/Jobs/Middleware/LocalizeJob.php:46)
  at Maatwebsite\Excel\Jobs\Middleware\LocalizeJob->Maatwebsite\Excel\Jobs\Middleware\{closure}()
     (/var/task/vendor/laravel/framework/src/Illuminate/Support/Traits/Localizable.php:19)
  at Maatwebsite\Excel\Jobs\Middleware\LocalizeJob->withLocale(null, object(Closure))
     (/var/task/vendor/maatwebsite/excel/src/Jobs/Middleware/LocalizeJob.php:45)
  at Maatwebsite\Excel\Jobs\Middleware\LocalizeJob->handle(object(QueueExport), object(Closure))
     (/var/task/vendor/maatwebsite/excel/src/Jobs/QueueExport.php:62)
JeRabix commented 1 week ago

I have this error in export:

Could not find zip member zip:///var/www/vhosts/laravel-project-folder/storage/framework/cache/laravel-excel/laravel-excel-2DORIebMAGG5N9kLneIvjORM66aBlFqv.xlsx#_rels\.rels

No octane, no vapor. I get an error only on a large amount of data, on a small one everything is ok.

I use fromQuery + queue.

Locale machine and stage project (the same server as the prod) no problem for 9-10k records.

But in production project - has this error with 6k, 10k records.

Type PhpOffice\PhpSpreadsheet\Reader\Exception
Location /var/www/vhosts/laravel-project/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Shared/File.php:162

Trace: trace

JeRabix commented 1 week ago

i add -d memory_limit=320M for my worker and now - all work good. This flag was originally on the stage - perhaps that's why everything worked stably there.

I also added a separate "exports" queue and started running exports on it instead of "defaullt" as before. I also reduced the size of chunks from a thousand to 300 pieces per chunk.

But I think the main problem was still in the memory limit