`RefCell` in the `TuiLogicalState`. Also some UI thought about what
history should be shared with what.
-## Error handling in the UI
-
-Real Monochrome has an 'Error Log' at ESC Y V, containing records of
-errors that happened in relation to your particular connection, such
-as when you tried to read a file but a backend error happened on the
-server. We should have one of those, and use it to handle failure to
-retrieve things you were trying to read.
-
-This is already marked in the code with multiple FIXMEs:
-
-* the `result.expect` at the end of `new_activity_state`
-* the handler for the existing `LogicalAction::Error`, which currently
- just beeps
-* the handler for the return value of `Client::process_stream_update`
-* the error in `PostMenu::post`, although if the error is recoverable
- (e.g. by editing the text and retrying) then the Error Log is
- overkill and we should redisplay the `PostMenu` with the message
-* errors in `get_user_to_examine` and `get_post_id_to_read`, although
- again if it's just 'no such thing' then the Error Log is overkill.
- Should only be for 'help, the server is totally confused'.
-* errors in `save_ldb`
-
-Each of those should instead catch the error, make an Error Log
-record, and transfer you to the error-log viewer with a beep, just the
-way Mono did it.
+## Error handling improvements
+
+Error Log entries could usefully include various extra detail:
+* `ClientError` should distinguish HTTP methods, so that the
+ inappropriate text 'fetching URL' isn't used in the case where you
+ were trying to _do_ something, like post a toot.
+* probably the method of `Client` that generated an error ought to
+ annotate it with a description of the client operation being
+ performed, so that it's not just 'while fetching this URL' but
+ 'while trying to retrieve a status / account / etc', and maybe even
+ which one.
+* the caller of that method in turn might also want to add further
+ context, like 'I was trying to fetch this status because this other
+ one was replying to it'.
+
+Some error situations could be handled in a less generic way:
+
+* in `PostMenu::post`, if the server refuses the post, we should
+ display the error in the posting menu and let the user modify and
+ retry, similar to the way the login/registration flow does it. But
+ any error weirder than that should still go to the Error Log.
+* similarly, in the `get_user_to_examine` and `get_post_id_to_read`
+ overlay activities, we should respond to 'no such thing' by letting
+ the user re-edit and try again, in case it was just a typo
+
+We don't have good handling for I/O errors while saving the user's
+LDB. That's not a _client_ error, but we could make an extra enum
+branch in `ClientError` anyway, so that the TuiLogicalState could put
+it in the Error Log to notify the user of a problem. Then perhaps we
+could set a flag to suppress further LDB-saving errors (don't want to
+keep whinging every 2 minutes or on every file change!), and clear the
+flag again if a later LDB save is successful (e.g. the user has freed
+up some disk space).
## Reading
ListStatusBoosters(String),
UserOptions(String),
InstanceRules,
+ ErrorLog,
}
#[derive(PartialEq, Eq, Debug, Clone, Hash)]
Boosters(String),
Followers(String),
Followees(String),
+ Errors,
}
#[derive(Debug, PartialEq, Eq, Clone)]
impl Client {
pub fn new(auth: AuthConfig) -> Result<Self, ClientError> {
- Ok(Client {
+ let mut client = Client {
auth,
client: reqwest_client()?,
accounts: HashMap::new(),
permit_write: false,
logfile: None,
error_log: ErrorLog::new(),
- })
+ };
+
+ client.feeds.insert(
+ FeedId::Errors,
+ Feed {
+ ids: VecDeque::new(),
+ origin: 0,
+ extend_past: None,
+ extend_future: None,
+ },
+ );
+
+ Ok(client)
}
pub fn set_writable(&mut self, permit: bool) {
FeedId::Followees(id) => {
Req::get(&format!("api/v1/accounts/{id}/following"))
}
+ FeedId::Errors => return Ok(false),
};
let req = match ext {
}
acs.iter().rev().map(|ac| ac.id.clone()).collect()
}
+ FeedId::Errors => {
+ panic!("we should have returned early in this case")
+ }
};
let any_new = !ids.is_empty();
}
pub fn add_to_error_log(&mut self, err: ClientError) {
+ let new_id =
+ self.error_log.origin + (self.error_log.items.len() as isize);
self.error_log.push_now(err);
+ self.feeds
+ .get_mut(&FeedId::Errors)
+ .expect("the Error feed should always be present")
+ .ids
+ .push_back(new_id.to_string());
+ }
+
+ pub fn get_error_log_entry(
+ &self,
+ id: &str,
+ ) -> (ClientError, DateTime<Utc>) {
+ let index = isize::from_str_radix(id, 10).expect(
+ "We generated these ids ourselves and they should all be valid",
+ );
+ self.error_log.items[(index - self.error_log.origin) as usize].clone()
}
}
}
fn extendable(&self) -> bool {
- true
+ self.id != FeedId::Errors
}
fn want_to_jump_to_new_content(&self) -> bool {
- self.id == FeedId::Mentions
+ self.id == FeedId::Mentions || self.id == FeedId::Errors
}
}
)?;
Ok(Box::new(file))
}
+
+struct ErrorLogFileType;
+impl FileType for ErrorLogFileType {
+ type Item = ErrorLogEntry;
+
+ fn get_item(
+ &self,
+ id: &str,
+ client: &mut Client,
+ ) -> Result<Self::Item, ClientError> {
+ let (err, time) = client.get_error_log_entry(id);
+ Ok(ErrorLogEntry::new(err, time))
+ }
+}
+
+pub fn error_log(
+ file_positions: &HashMap<FeedId, SavedFilePos>,
+ client: &mut Client,
+ is_interrupt: bool,
+) -> Box<dyn ActivityState> {
+ let feed = FeedId::Errors;
+ let pos = file_positions.get(&feed);
+ let title = ColouredString::general(
+ "Error Log [ESC][Y][V]",
+ "HHHHHHHHHHHHHKKKHHKHHKH",
+ );
+
+ let file = File::new_infallible(
+ client,
+ FeedSource::new(feed),
+ title,
+ ErrorLogFileType,
+ pos,
+ None,
+ is_interrupt,
+ );
+ Box::new(file)
+}
Err(err) => {
self.client.add_to_error_log(err);
- // FIXME: throw user into the Error Log
+ self.state.throw_into_error_log(&mut self.client);
Vec::new()
}
}
}
LogicalAction::Error(err) => {
client.add_to_error_log(err);
+ self.activity_stack.goto(UtilityActivity::ErrorLog.into());
+ self.changed_activity(client, None, true);
break PhysicalAction::Beep;
}
LogicalAction::PostComposed(post) => {
client,
is_interrupt,
),
+ Activity::Util(UtilityActivity::ErrorLog) => {
+ Ok(error_log(&self.file_positions, client, is_interrupt))
+ }
Activity::Util(UtilityActivity::EgoLog) => {
ego_log(&self.file_positions, client)
}
Ok(state) => state,
Err(err) => {
client.add_to_error_log(err);
- panic!("FIXME: need to implement the Error Log here");
+ error_log(&self.file_positions, client, true)
}
}
}
None => None,
}
}
+
+ fn throw_into_error_log(&mut self, client: &mut Client) {
+ self.activity_stack.goto(UtilityActivity::ErrorLog.into());
+ self.changed_activity(client, None, true);
+ }
}
~ian / mastodonochrome.git / commitdiff