Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: ErrorException: serialize(): "spreadsheet" returned as member variable from __sleep() but does not exist when queueing an import #4157

Closed
1 task done
ChrisExP opened this issue Jun 28, 2024 · 5 comments

Comments

@ChrisExP
Copy link

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

  • Yes, it's still reproducable

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.

@ChrisExP ChrisExP added the bug label Jun 28, 2024
@patrickbrouwers
Copy link
Member

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
Copy link

zamacona commented Sep 6, 2024

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
Copy link
Member

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

@phatnguyenea
Copy link

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

@stale stale bot added the stale label Nov 16, 2024
Copy link

stale bot commented Nov 17, 2024

This bug report has been automatically closed because it has not had recent activity. If this is still an active bug, please comment to reopen. Thank you for your contributions.

@stale stale bot closed this as completed Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants