Skip to content
New issue

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

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

Already on GitHub? # to your account

fix the execution order of brotli class #2701

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Sep 12, 2024

Found the bug. It seems the execution order was wrong. I couldn't find a proper way to test this, but manually running the tests show no uncaught exception.

Fixes #2694

@anonrig anonrig requested review from jasnell and npaun September 12, 2024 18:55
@anonrig anonrig requested review from a team as code owners September 12, 2024 18:55
@anonrig
Copy link
Member Author

anonrig commented Sep 12, 2024

It seems this error is only visible on the logs, since error is thrown on the queueMicrotask call.

workerd/io/worker.c++:1054: info: uncaught exception; desc = Error: Setting parameter failed

@npaun
Copy link
Member

npaun commented Sep 12, 2024

@anonrig Yep you found what I was talking about

@anonrig anonrig force-pushed the yagiz/regression-test-for-createbrotli branch from fe88b1a to b122dae Compare September 12, 2024 20:01
@anonrig anonrig requested a review from a team as a code owner September 12, 2024 20:01
@anonrig anonrig changed the title test createBrotliCompress error fix the execution order of brotli class Sep 12, 2024
@anonrig
Copy link
Member Author

anonrig commented Sep 12, 2024

ok, i think i found it. we should have called the parent class constructor after we initialize the handle.

@anonrig anonrig merged commit b4d69b5 into main Sep 12, 2024
13 checks passed
@anonrig anonrig deleted the yagiz/regression-test-for-createbrotli branch September 12, 2024 20:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zlib.createBrotliCompress emits unexpected error
2 participants