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

Fix safari+firefox with service worker: clone file.encodedName on yield #83

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

zakstucke
Copy link
Contributor

@zakstucke zakstucke commented Apr 18, 2024

Love the library. I encountered a very weird issue i painstakingly debugged, resulting in this PR.

Setup:

Error occurs on closing the zip (i.e. once all files have been included and the stream is being closed by client-zip)
Firefox error:
MessagePort.postMessage: attempting to access detached ArrayBuffer

Safari error:
Unhandled Promise Rejection: DataCloneError: The object can not be cloned.

The firefox error is the actual cause, the safari one gets hidden behind the above opaque one.

What I think is happening and why this PR fixes it:

  • When service worker is being used, the buffers get transferred to the service worker.
  • Not sure on exactly how the zipping works, but it looks like file.encodedName ends up getting sent twice during the zipping process, probably with the file, and in the zip's manifest
  • With service worker, means needs to be transferred twice, therefore this error occurs as the buffers already been transferred.
  • So by cloning the buffer on the first send, the same buffer will no longer be transferred twice, and no issue!

This change works for me now on both safari and firefox.

Thanks!

@Touffy
Copy link
Owner

Touffy commented Apr 19, 2024

Your analysis is correct. The file name buffer is used twice in the stream — in the file header and the central repository (the "index" of the archive) — and of course it can't be transferred twice (which is happening because you're running client-zip in the client window, and the Native File System adapter is transferring the stream to the Service Worker).

@Touffy Touffy merged commit 8e59d0c into Touffy:master Apr 19, 2024
1 check passed
@zakstucke
Copy link
Contributor Author

Great. Thanks!

@Touffy
Copy link
Owner

Touffy commented Apr 19, 2024

I'll publish this evening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants