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]: ErrorException: serialize(): "spreadsheet" returned as member variable from __sleep() but does not exist when queueing an import #4157

Open ChrisExP opened 4 months ago

ChrisExP commented 4 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.3.55

What version of Laravel are you using?

10.48.10

What version of PHP are you using?

8.3.4

Describe your issue

An exception is thrown when running two imports in the same process.
The first one needs to be a chunked import without queueing.
The second needs to be a chunked import with queueing.

When these two imports are run, the second one fails when it tries to serialize the AfterImportJob class.

The error message looks like this:

serialize(): "spreadsheet" returned as member variable from __sleep() but does not exist

How can the issue be reproduced?

I have created a repo that includes a test which triggers the exception: https://github.com/ChrisExP/ExcelQueueIssue

But you can reproduce it any Laravel project with the following code:

$file = getImportFile();
$regularImport = new RegularImport(); // An import class that implements WithChunkReading
$queuedImport = new QueuedImport(); // An import class that implements WithChunkReading and ShouldQueue
Excel::import($regularImport, $file);
// Excel::clearResolvedInstances(); // Uncommenting this will make the test pass
Excel::import($queuedImport, $file);

I'm not sure why the order is relevant or why one needs to be queued and the other not queued, but this is the only order that triggers the issue.

What should be the expected behaviour?

Both imports should complete successfully, one synchronously in the same process and one as a job.

I believe the issue is related to the garbageCollect method in Maatwebsite\Excel\Reader.
That method unsets the spreadsheet and sheetImports properties on the class, so when the reader is invoked again and tries to serialize, those properties do not exist.

If you clear the Excel facade between uses then it fixes the issue because it instantiates a new Excel instance with a new Reader class, and that has spreadsheet set to null, even though it hasn't been initialized yet.

It is unclear why it is fine serializing an uninitialized property but has trouble with an unset property, but maybe that's just a quirk with PHP.

With some experimenting I found that setting $this->spreadsheet and $this->sheetImports to null fixes the issue, but I'm not sure if that might cause other issues.

patrickbrouwers commented 2 months ago

I don't think I ever intended multiple imports to be done after each other, so the garbage collect method expects that it's the end of the import so it can free up the memory. I guess that there should be a check somewhere that checks if spreadsheet is not set, it will re-create the dependency.

zamacona commented 1 month ago

I solved it by using Excel::clearResolvedInstances(); after calling Excel::import(), but I don't know if it's the right way. I think it shouldn't be necessary.

patrickbrouwers commented 1 month ago

I think it's the best way for now. Will see if we can do something to support it in the future

phatnguyenea commented 1 month ago

I solved it by using Excel::clearResolvedInstances(); after calling Excel::import(), but I don't know if it's the right way. I think it shouldn't be necessary.

The best way for me, many thanks