-
Notifications
You must be signed in to change notification settings - Fork 249
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
Update queues debug logging to ignore aborted requests #1773
Update queues debug logging to ignore aborted requests #1773
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in chat I believe we can get the script ID using incomingRequest->getContext().getWorker().getScript().getId()
, but I'm not seeing an existing way to get the owner ID other than either plumbing it into the QueueEvent
or returning the error back to the caller in some way for it to do the logging.
@@ -91,7 +91,8 @@ class IoContext_IncomingRequest { | |||
// This method is also used by some custom event handlers (see WorkerInterface::CustomEvent) that | |||
// need similar behavior, as well as the test handler. TODO(cleanup): Rename to something more | |||
// generic? | |||
kj::Promise<bool> finishScheduled(); | |||
enum class FinishScheduledResult { COMPLETED, ABORTED, TIMEOUT }; | |||
kj::Promise<FinishScheduledResult> finishScheduled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the return value here will also need a parallel change to the internal code since there are a couple callers of this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I've got that all wired up, just haven't put it up for PR yet.
To be clear, hopefully I've just missed something, but it doesn't look promising. |
933a9d8
to
db27270
Compare
db27270
to
084b006
Compare
Two force pushes:
|
WIP
CC: @a-robinson