Skip to content

Commit 5c9651c

Browse files
committed
EW-9366 Report Return event for non-fetch events too
1 parent d0ab9e6 commit 5c9651c

File tree

10 files changed

+80
-29
lines changed

10 files changed

+80
-29
lines changed

src/workerd/api/global-scope.c++

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <workerd/io/compatibility-date.h>
1919
#include <workerd/io/features.h>
2020
#include <workerd/io/io-context.h>
21+
#include <workerd/io/tracer.h>
2122
#include <workerd/jsg/async-context.h>
2223
#include <workerd/jsg/ser.h>
2324
#include <workerd/jsg/util.h>
@@ -396,7 +397,12 @@ void ServiceWorkerGlobalScope::startScheduled(kj::Date scheduledTime,
396397
KJ_IF_SOME(h, exportedHandler) {
397398
KJ_IF_SOME(f, h.scheduled) {
398399
auto promise =
399-
f(lock, js.alloc<ScheduledController>(event.addRef()), h.env.addRef(isolate), h.getCtx());
400+
f(lock, js.alloc<ScheduledController>(event.addRef()), h.env.addRef(isolate), h.getCtx())
401+
.then([&context]() {
402+
KJ_IF_SOME(t, context.getWorkerTracer()) {
403+
t.setReturn(context.now());
404+
}
405+
});
400406
event->waitUntil(kj::mv(promise));
401407
} else {
402408
lock.logWarningOnce(
@@ -623,7 +629,9 @@ kj::Promise<void> ServiceWorkerGlobalScope::setHibernatableEventTimeout(
623629
return event;
624630
}
625631

626-
void ServiceWorkerGlobalScope::sendHibernatableWebSocketMessage(
632+
// TODO(cleanup): the hibernatable websocket handler functions here are largely identical – consider
633+
// folding them.
634+
void ServiceWorkerGlobalScope::sendHibernatableWebSocketMessage(IoContext& context,
627635
kj::OneOf<kj::String, kj::Array<byte>> message,
628636
kj::Maybe<uint32_t> eventTimeoutMs,
629637
kj::String websocketId,
@@ -637,13 +645,19 @@ void ServiceWorkerGlobalScope::sendHibernatableWebSocketMessage(
637645
KJ_IF_SOME(h, exportedHandler) {
638646
KJ_IF_SOME(handler, h.webSocketMessage) {
639647
event->waitUntil(setHibernatableEventTimeout(
640-
handler(lock, kj::mv(websocket), kj::mv(message)), eventTimeoutMs));
648+
handler(lock, kj::mv(websocket), kj::mv(message)), eventTimeoutMs)
649+
.then([&context]() {
650+
KJ_IF_SOME(t, context.getWorkerTracer()) {
651+
t.setReturn(context.now());
652+
}
653+
}));
641654
}
642655
// We want to deliver a message, but if no webSocketMessage handler is exported, we shouldn't fail
643656
}
644657
}
645658

646-
void ServiceWorkerGlobalScope::sendHibernatableWebSocketClose(HibernatableSocketParams::Close close,
659+
void ServiceWorkerGlobalScope::sendHibernatableWebSocketClose(IoContext& context,
660+
HibernatableSocketParams::Close close,
647661
kj::Maybe<uint32_t> eventTimeoutMs,
648662
kj::String websocketId,
649663
Worker::Lock& lock,
@@ -663,13 +677,19 @@ void ServiceWorkerGlobalScope::sendHibernatableWebSocketClose(HibernatableSocket
663677
KJ_IF_SOME(handler, h.webSocketClose) {
664678
event->waitUntil(setHibernatableEventTimeout(
665679
handler(lock, kj::mv(websocket), close.code, kj::mv(close.reason), close.wasClean),
666-
eventTimeoutMs));
680+
eventTimeoutMs)
681+
.then([&context]() {
682+
KJ_IF_SOME(t, context.getWorkerTracer()) {
683+
t.setReturn(context.now());
684+
}
685+
}));
667686
}
668687
// We want to deliver close, but if no webSocketClose handler is exported, we shouldn't fail
669688
}
670689
}
671690

672-
void ServiceWorkerGlobalScope::sendHibernatableWebSocketError(kj::Exception e,
691+
void ServiceWorkerGlobalScope::sendHibernatableWebSocketError(IoContext& context,
692+
kj::Exception e,
673693
kj::Maybe<uint32_t> eventTimeoutMs,
674694
kj::String websocketId,
675695
Worker::Lock& lock,
@@ -689,7 +709,12 @@ void ServiceWorkerGlobalScope::sendHibernatableWebSocketError(kj::Exception e,
689709
KJ_IF_SOME(h, exportedHandler) {
690710
KJ_IF_SOME(handler, h.webSocketError) {
691711
event->waitUntil(setHibernatableEventTimeout(
692-
handler(js, kj::mv(websocket), js.exceptionToJs(kj::mv(e))), eventTimeoutMs));
712+
handler(js, kj::mv(websocket), js.exceptionToJs(kj::mv(e))), eventTimeoutMs)
713+
.then([&context]() {
714+
KJ_IF_SOME(t, context.getWorkerTracer()) {
715+
t.setReturn(context.now());
716+
}
717+
}));
693718
}
694719
// We want to deliver an error, but if no webSocketError handler is exported, we shouldn't fail
695720
}

src/workerd/api/global-scope.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -510,19 +510,22 @@ class ServiceWorkerGlobalScope: public WorkerGlobalScope {
510510
kj::Promise<void> setHibernatableEventTimeout(
511511
kj::Promise<void> event, kj::Maybe<uint32_t> eventTimeoutMs);
512512

513-
void sendHibernatableWebSocketMessage(kj::OneOf<kj::String, kj::Array<byte>> message,
513+
void sendHibernatableWebSocketMessage(IoContext& context,
514+
kj::OneOf<kj::String, kj::Array<byte>> message,
514515
kj::Maybe<uint32_t> eventTimeoutMs,
515516
kj::String websocketId,
516517
Worker::Lock& lock,
517518
kj::Maybe<ExportedHandler&> exportedHandler);
518519

519-
void sendHibernatableWebSocketClose(HibernatableSocketParams::Close close,
520+
void sendHibernatableWebSocketClose(IoContext& context,
521+
HibernatableSocketParams::Close close,
520522
kj::Maybe<uint32_t> eventTimeoutMs,
521523
kj::String websocketId,
522524
Worker::Lock& lock,
523525
kj::Maybe<ExportedHandler&> exportedHandler);
524526

525-
void sendHibernatableWebSocketError(kj::Exception e,
527+
void sendHibernatableWebSocketError(IoContext& context,
528+
kj::Exception e,
526529
kj::Maybe<uint32_t> eventTimeoutMs,
527530
kj::String websocketId,
528531
Worker::Lock& lock,

src/workerd/api/hibernatable-web-socket.c++

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,22 +85,24 @@ kj::Promise<WorkerInterface::CustomEvent::Result> HibernatableWebSocketCustomEve
8585
props = kj::mv(props)](Worker::Lock& lock) mutable {
8686
KJ_SWITCH_ONEOF(eventParameters.eventType) {
8787
KJ_CASE_ONEOF(text, HibernatableSocketParams::Text) {
88-
return lock.getGlobalScope().sendHibernatableWebSocketMessage(kj::mv(text.message),
89-
eventParameters.eventTimeoutMs, kj::mv(eventParameters.websocketId), lock,
88+
return lock.getGlobalScope().sendHibernatableWebSocketMessage(context,
89+
kj::mv(text.message), eventParameters.eventTimeoutMs,
90+
kj::mv(eventParameters.websocketId), lock,
9091
lock.getExportedHandler(entrypointName, kj::mv(props), context.getActor()));
9192
}
9293
KJ_CASE_ONEOF(data, HibernatableSocketParams::Data) {
93-
return lock.getGlobalScope().sendHibernatableWebSocketMessage(kj::mv(data.message),
94-
eventParameters.eventTimeoutMs, kj::mv(eventParameters.websocketId), lock,
94+
return lock.getGlobalScope().sendHibernatableWebSocketMessage(context,
95+
kj::mv(data.message), eventParameters.eventTimeoutMs,
96+
kj::mv(eventParameters.websocketId), lock,
9597
lock.getExportedHandler(entrypointName, kj::mv(props), context.getActor()));
9698
}
9799
KJ_CASE_ONEOF(close, HibernatableSocketParams::Close) {
98-
return lock.getGlobalScope().sendHibernatableWebSocketClose(kj::mv(close),
100+
return lock.getGlobalScope().sendHibernatableWebSocketClose(context, kj::mv(close),
99101
eventParameters.eventTimeoutMs, kj::mv(eventParameters.websocketId), lock,
100102
lock.getExportedHandler(entrypointName, kj::mv(props), context.getActor()));
101103
}
102104
KJ_CASE_ONEOF(e, HibernatableSocketParams::Error) {
103-
return lock.getGlobalScope().sendHibernatableWebSocketError(kj::mv(e.error),
105+
return lock.getGlobalScope().sendHibernatableWebSocketError(context, kj::mv(e.error),
104106
eventParameters.eventTimeoutMs, kj::mv(eventParameters.websocketId), lock,
105107
lock.getExportedHandler(entrypointName, kj::mv(props), context.getActor()));
106108
}

src/workerd/api/queue.c++

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,7 @@ struct StartQueueEventResponse {
470470
};
471471

472472
StartQueueEventResponse startQueueEvent(EventTarget& globalEventTarget,
473+
IoContext& context,
473474
kj::OneOf<rpc::EventDispatcher::QueueParams::Reader, QueueEvent::Params> params,
474475
IoPtr<QueueEventResult> result,
475476
Worker::Lock& lock,
@@ -493,8 +494,11 @@ StartQueueEventResponse startQueueEvent(EventTarget& globalEventTarget,
493494
KJ_IF_SOME(f, queueHandler.queue) {
494495
auto promise = f(lock, js.alloc<QueueController>(event.addRef()),
495496
jsg::JsValue(h.env.getHandle(js)).addRef(js), h.getCtx())
496-
.then([event = event.addRef()]() mutable {
497+
.then([event = event.addRef(), &context]() mutable {
497498
event->setCompletionStatus(QueueEvent::CompletedSuccessfully{});
499+
KJ_IF_SOME(t, context.getWorkerTracer()) {
500+
t.setReturn(context.now());
501+
}
498502
}, [event = event.addRef()](kj::Exception&& e) mutable {
499503
event->setCompletionStatus(QueueEvent::CompletedWithError{kj::cp(e)});
500504
return kj::mv(e);
@@ -572,7 +576,7 @@ kj::Promise<WorkerInterface::CustomEvent::Result> QueueCustomEventImpl::run(
572576
jsg::AsyncContextFrame::StorageScope traceScope = context.makeAsyncTraceScope(lock);
573577

574578
auto& typeHandler = lock.getWorker().getIsolate().getApi().getQueueTypeHandler(lock);
575-
auto startResp = startQueueEvent(lock.getGlobalScope(), kj::mv(params),
579+
auto startResp = startQueueEvent(lock.getGlobalScope(), context, kj::mv(params),
576580
context.addObject(result), lock,
577581
lock.getExportedHandler(entrypointName, kj::mv(props), context.getActor()), typeHandler);
578582
queueEvent->event = kj::mv(startResp.event);

src/workerd/api/tail-worker-test-receiver.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const test = {
3434
// Number of traces based on how often main tail worker is invoked from previous tests
3535
let numTraces = 28;
3636
let basicTrace =
37-
'{"type":"onset","executionModel":"stateless","spanId":"0000000000000000","scriptTags":[],"info":{"type":"trace","traces":[]}}{"type":"outcome","outcome":"ok","cpuTime":0,"wallTime":0}';
37+
'{"type":"onset","executionModel":"stateless","spanId":"0000000000000000","scriptTags":[],"info":{"type":"trace","traces":[]}}{"type":"return"}{"type":"outcome","outcome":"ok","cpuTime":0,"wallTime":0}';
3838
assert.deepStrictEqual(
3939
Array.from(resposeMap.values()),
4040
Array.from({ length: numTraces }, () => basicTrace)

0 commit comments

Comments
 (0)