fix: add severity to user function error reporting #393
Conversation
github-actions
bot
added
javascript-sdk
kalix-runtime
| /** @internal */ | ||
| enum UserFunctionErrorSeverity { | ||
| UNSPECIFIED = 0, | ||
| ERROR = 1, | ||
| WARNING = 2, | ||
| INFO = 3, | ||
| } |
There was a problem hiding this comment.
Forgot that the protocol types can only be used in type position. Using an enum compiles in a reference to it, but these types are not packaged in the dist. So mirror the severity enum here for switches. Seems a bit awkward, so maybe the protocol typing and whether that gets included in the package, should be revisited.
sdk/src/kalix.ts
Outdated
| case UserFunctionErrorSeverity.INFO: | ||
| case UserFunctionErrorSeverity.WARNING: | ||
| console.log(msg); |
There was a problem hiding this comment.
There is a console.warn but it's an alias for console.error, which prints to stderr. This puts warnings on stdout with console.log instead, as GCP logging will mark stderr as ERRORs. But maybe having it use console.warn and print to stderr is better anyway?
There was a problem hiding this comment.
If we don't have any more fine grained output then I think console.warn/stderr makes sense.
There was a problem hiding this comment.
Ok, will update to use console.{info,warn,error}. And we can then switch to a proper logging layer later, with structured JSON output. Follow up issue will be https://github.com/lightbend/kalix-proxy/issues/618.
|
Rebased and ready for review now. |
sdk/src/kalix.ts
Outdated
| case UserFunctionErrorSeverity.INFO: | ||
| case UserFunctionErrorSeverity.WARNING: | ||
| console.log(msg); |
There was a problem hiding this comment.
If we don't have any more fine grained output then I think console.warn/stderr makes sense.
| break; | ||
| } | ||
|
|
||
| let msg = `${messageType} reported from Kalix system: ${code} ${message}`; |
There was a problem hiding this comment.
BTW, shouldn't this be JSON anyway?
There was a problem hiding this comment.
Right, no structured JSON logging anywhere at the moment. Older issue is https://github.com/lightbend/kalix-proxy/issues/618. Could make sense to have our own small logging API, that users can also make use of, from the context or something.
Refs https://github.com/lightbend/kalix-proxy/issues/1566. Builds on #392.