Conversation
| LOG_ERROR_PERIODICALLY("NOSENTRY worker is not an actor but class name was requested", n); | ||
| } else { | ||
| LOG_ERROR_PERIODICALLY("worker has no such named entrypoint", n); | ||
| LOG_ERROR_PERIODICALLY("NOSENTRY worker has no such named entrypoint", n); |
There was a problem hiding this comment.
The ideal here would be to not log this at all specifically for cross-account dynamic dispatch (or maybe for all dynamic dispatch), rather than not sending it to sentry in any case. The reasoning being that in that case it's the caller's fault rather than being something wrong with the runtime / validator / config APIs.
I imagine the original intent of this log was to catch issues where the config APIs managed to configure a route/trigger to a worker with an incorrect named entrypoint, and that case is still a real error worth raising to sentry if it happens.
If it's too hard to differentiate here then this is better than letting the log be noisy, but I wanted to clarify in case the change was done this way was based on a misunderstanding rather than due to the ideal version being too hard.
There was a problem hiding this comment.
I wholly agree here.
Generally we should shy away from NOSENTRY as much as possible.
If something is a user error it shouldn't be logged at all BUT it should be a good user error - jsg.Error
If something is a platform error it should be in sentry.
This sounds like it should be converted to a jsg.Error under dynamic dispatch regardless of cross account right?
There was a problem hiding this comment.
Ah, got it. I'd missed that bit of context. I had interpreted this as a thing we want to watch for (log) but not necessarily track or alert on (sentry)... Hadn't connected the " no, this is an error for the user to do something about" dots mentally. In that case, absolutely agree
There was a problem hiding this comment.
This sounds like it should be converted to a jsg.Error under dynamic dispatch regardless of cross account right?
Right.
|
FYI, I made these errors user-visible for dynamic dispatch only in #6400 |
These are caused by user misconfiguration (e.g. Workflows cross-account dynamic dispatch with a missing entrypoint) and shouldn't go to Sentry.