fix: SDK analytics #545
fix: SDK analytics #545
Conversation
chris13524
force-pushed
the
fix/sdk-analytics
branch
from
d80298c to
be38431
Compare
|
|
||
| request_auth | ||
| .validate() | ||
| .map_err(RelayMessageServerError::NotifyServer)?; // TODO change to client error? |
There was a problem hiding this comment.
Can you elaborate on why it should be a client error?
There was a problem hiding this comment.
I have a bunch of these TODOs throughout the codebase, basically means "you should re-evaluate if you can do it better". Started when I started refactoring stuff to be capable of sending errors to clients but there are still a number of places remaining.
In this case, it should be a client error because the request failed validation.
There was a problem hiding this comment.
Saw the sends via the channel fed to handle, didn't strike me as too weird but I may lack context.
FWIW in the Go programming language you'd normally spawn goroutines (tokio tasks) and expect them to send back data via channels so this concurrency setup is not entirely foreign
This reverts commit afad3cc.
Description
Adds SDK version analytics as per the spec.
This adds a oneshot channel to transmit this information to outside of the
handle()function which is a little strange, but seemed like a good enough solution considering the desire to have this info for bothOkandErrcases and it would be more verbose to make returningsdkin both cases compatible with?.This also moves the validation of the JWT payloads further up in the request processing.
Resolves #475
How Has This Been Tested?
Not tested
Due Diligence