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

Log Actor ID on V8 deserialization errors #1354

Merged
merged 5 commits into from
Oct 27, 2023

Conversation

justin-mp
Copy link
Contributor

When V8 deserialization fails, it’s likely due to corruption somewhere
between the Actor in JS and the storage layer. In order to aid
debugging, log the actor id if we know of one.

I’m going to want to put things in the SetupParams struct that must be
moved out in order to be used.  As such, some of the things in the
params member would be valid and some things would be invalid after
running the constructor.  Rather than have to document this mess, it’s
better to treat the SetupParams as something only the constructor has
access to and save away everything else we need other data member.
@justin-mp justin-mp requested review from a team as code owners October 26, 2023 20:00
@github-actions
Copy link

github-actions bot commented Oct 26, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

Copy link
Contributor

@bcaimano bcaimano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, seems reasonable to me!

@justin-mp
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@justin-mp
Copy link
Contributor Author

recheck

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming CI is green

I will need to put things in the SetupParams struct that are only
moveable.  As such, we’ll want the params parameter to be an rvalue
reference so the compiler can give us hints to move the struct into
place.
I’m going to want to use this to make an Actor stub in a test.
I need to test something that requires IoContexts that have Actors
associated with them.  The easiest way to do this was to extend the
TestFixture to create a stubbed out Actor for us.
When V8 deserialization fails, it’s likely due to corruption somewhere
between the Actor in JS and the storage layer.  In order to aid
debugging, log the actor id if we know of one.
@justin-mp
Copy link
Contributor Author

be056d0 included in the push should fix the CI problems.

@justin-mp justin-mp merged commit 7448549 into cloudflare:main Oct 27, 2023
11 checks passed
@justin-mp justin-mp deleted the log-actor-on-error branch October 27, 2023 11:55
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.

3 participants