Authentication #37
Authentication #37
Conversation
| /// To run this, you need to first spawn this function as a task, then | ||
| /// open a browser to the given URL and finally wait on the spawned future | ||
| /// with the ability to cancel in case the browser is closed before finishing | ||
| #[tracing::instrument] |
There was a problem hiding this comment.
are these macros needed everywhere with this lib?
There was a problem hiding this comment.
Only on functions which need to be known about in the spantrace. Unfortunately, since async Rust doesn't have any real consistent stack to trace usefully, we need to use the macro to determine where we are in actual control flow.
| #[derive(thiserror::Error, Debug)] | ||
| pub enum Error { | ||
| pub enum ErrorKind { |
There was a problem hiding this comment.
What's the purpose of this name change? Most Rust libs use Error, and I think we should stick to that
There was a problem hiding this comment.
It is now integrated into the actual error type, which captures it and a spantrace so it can be traced back further than where application code is invoked.
There was a problem hiding this comment.
If there were a better solution for async, the authors of the Tokio runtime probably would have used it as the main way to trace errors in their own runtime (after all, Tokio is the one who authored the tracing library and is using it in their console debugger).
There was a problem hiding this comment.
IMO the old Error should be called Error because it's the direct library error. The other error should be TracableError or something of the sorts
There was a problem hiding this comment.
Actually, everywhere uses this kind of convention, backtraces are captured using a similar pattern usually.
| @@ -52,4 +69,35 @@ pub enum Error { | |||
| OtherError(String), | |||
| } | |||
|
|
|||
| #[derive(Debug)] | |||
| pub struct Error { | |||
There was a problem hiding this comment.
What's the purpose of this name change? Most Rust libs use
Error, and I think we should stick to that
@Geometrically - This is the new error type, which wraps ErrorKind with the spantrace so that errors can be traced further than the call site.
|
To be honest, I don't like the whole idea of tracing/implementing this library. I'm not sure what issues it solves. It adds a lot of extra code, and I don't think this will go well when we have to implement it in tauri |
It allows errors to know where they came from, in async the stack is not very helpful in tracing errors since each task is managed by an executor which is detached from the call stack (this is the whole reason behind the need to move values into spawned tasks, since tasks can’t store data that could be popped off of the call stack before it finishes), thus requiring some other way to track position in control flow. As an example, this is an error without tracing And with tracing, it is: Enabling the backtrace gives something almost completely useless due to the aforementioned stack decoupling, so control flow needs to be tracked some other way. The macros provide a convenient way to attach a span to a function (though doing so manually is possible with tracing, allowing large functions to be split further for error tracing purposes), provides a way to only trace what functions are useful to know about, and allows you to customize how error reporting is handled. Tracing allows for the extraction of spans from types which track them, so the frontend could pretty easily extract the info from errors to provide usable error messages that can actually be used to find backend bugs. |
Intro
This integrates Hydra with Theseus to provide what can be described as the bare minimum requirement for a non-cracked launcher.
Closes #4
Tasks