Log prettier errors on failures (#19951)

Closes https://github.com/zed-industries/zed/issues/11987

Release Notes:

- Fixed prettier not reporting failures in the status panel on
formatting and installation errors
This commit is contained in:
Kirill Bulatov 2024-10-30 14:49:47 +02:00 committed by GitHub
parent 0ba40bdfb8
commit d49cd0019f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 146 additions and 85 deletions

1
Cargo.lock generated
View file

@ -16,6 +16,7 @@ dependencies = [
"project",
"smallvec",
"ui",
"util",
"workspace",
]

View file

@ -23,6 +23,7 @@ language.workspace = true
project.workspace = true
smallvec.workspace = true
ui.workspace = true
util.workspace = true
workspace.workspace = true
[dev-dependencies]

View file

@ -13,7 +13,8 @@ use language::{
use project::{EnvironmentErrorMessage, LanguageServerProgress, Project, WorktreeId};
use smallvec::SmallVec;
use std::{cmp::Reverse, fmt::Write, sync::Arc, time::Duration};
use ui::{prelude::*, ButtonLike, ContextMenu, PopoverMenu, PopoverMenuHandle};
use ui::{prelude::*, ButtonLike, ContextMenu, PopoverMenu, PopoverMenuHandle, Tooltip};
use util::truncate_and_trailoff;
use workspace::{item::ItemHandle, StatusItemView, Workspace};
actions!(activity_indicator, [ShowErrorMessage]);
@ -446,6 +447,8 @@ impl ActivityIndicator {
impl EventEmitter<Event> for ActivityIndicator {}
const MAX_MESSAGE_LEN: usize = 50;
impl Render for ActivityIndicator {
fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
let result = h_flex()
@ -456,6 +459,7 @@ impl Render for ActivityIndicator {
return result;
};
let this = cx.view().downgrade();
let truncate_content = content.message.len() > MAX_MESSAGE_LEN;
result.gap_2().child(
PopoverMenu::new("activity-indicator-popover")
.trigger(
@ -464,7 +468,21 @@ impl Render for ActivityIndicator {
.id("activity-indicator-status")
.gap_2()
.children(content.icon)
.child(Label::new(content.message).size(LabelSize::Small))
.map(|button| {
if truncate_content {
button
.child(
Label::new(truncate_and_trailoff(
&content.message,
MAX_MESSAGE_LEN,
))
.size(LabelSize::Small),
)
.tooltip(move |cx| Tooltip::text(&content.message, cx))
} else {
button.child(Label::new(content.message).size(LabelSize::Small))
}
})
.when_some(content.on_click, |this, handler| {
this.on_click(cx.listener(move |this, _, cx| {
handler(this, cx);

View file

@ -329,11 +329,7 @@ impl Prettier {
})?
.context("prettier params calculation")?;
let response = local
.server
.request::<Format>(params)
.await
.context("prettier format request")?;
let response = local.server.request::<Format>(params).await?;
let diff_task = buffer.update(cx, |buffer, cx| buffer.diff(response.text, cx))?;
Ok(diff_task.await)
}

View file

@ -29,6 +29,7 @@ use gpui::{
Task, WeakModel,
};
use http_client::HttpClient;
use itertools::Itertools as _;
use language::{
language_settings::{
language_settings, FormatOnSave, Formatter, LanguageSettings, SelectedFormatter,
@ -144,7 +145,6 @@ pub struct LocalLspStore {
HashMap<LanguageServerId, (LanguageServerName, Arc<LanguageServer>)>,
prettier_store: Model<PrettierStore>,
current_lsp_settings: HashMap<LanguageServerName, LspSettings>,
last_formatting_failure: Option<String>,
_subscription: gpui::Subscription,
}
@ -563,9 +563,7 @@ impl LocalLspStore {
})?;
prettier_store::format_with_prettier(&prettier, &buffer.handle, cx)
.await
.transpose()
.ok()
.flatten()
.transpose()?
}
Formatter::External { command, arguments } => {
Self::format_via_external_command(buffer, command, arguments.as_deref(), cx)
@ -705,6 +703,7 @@ impl LspStoreMode {
pub struct LspStore {
mode: LspStoreMode,
last_formatting_failure: Option<String>,
downstream_client: Option<(AnyProtoClient, u64)>,
nonce: u128,
buffer_store: Model<BufferStore>,
@ -907,7 +906,6 @@ impl LspStore {
language_server_watcher_registrations: Default::default(),
current_lsp_settings: ProjectSettings::get_global(cx).lsp.clone(),
buffers_being_formatted: Default::default(),
last_formatting_failure: None,
prettier_store,
environment,
http_client,
@ -917,6 +915,7 @@ impl LspStore {
this.as_local_mut().unwrap().shutdown_language_servers(cx)
}),
}),
last_formatting_failure: None,
downstream_client: None,
buffer_store,
worktree_store,
@ -977,6 +976,7 @@ impl LspStore {
upstream_project_id: project_id,
}),
downstream_client: None,
last_formatting_failure: None,
buffer_store,
worktree_store,
languages: languages.clone(),
@ -5265,9 +5265,9 @@ impl LspStore {
.map(language::proto::serialize_transaction),
})
}
pub fn last_formatting_failure(&self) -> Option<&str> {
self.as_local()
.and_then(|local| local.last_formatting_failure.as_deref())
self.last_formatting_failure.as_deref()
}
pub fn environment_for_buffer(
@ -5338,23 +5338,16 @@ impl LspStore {
cx.clone(),
)
.await;
lsp_store.update(&mut cx, |lsp_store, _| {
let local = lsp_store.as_local_mut().unwrap();
match &result {
Ok(_) => local.last_formatting_failure = None,
Err(error) => {
local.last_formatting_failure.replace(error.to_string());
}
}
lsp_store.update_last_formatting_failure(&result);
})?;
result
})
} else if let Some((client, project_id)) = self.upstream_client() {
let buffer_store = self.buffer_store();
cx.spawn(move |_, mut cx| async move {
let response = client
cx.spawn(move |lsp_store, mut cx| async move {
let result = client
.request(proto::FormatBuffers {
project_id,
trigger: trigger as i32,
@ -5365,13 +5358,21 @@ impl LspStore {
})
.collect::<Result<_>>()?,
})
.await?
.transaction
.ok_or_else(|| anyhow!("missing transaction"))?;
.await
.and_then(|result| result.transaction.context("missing transaction"));
lsp_store.update(&mut cx, |lsp_store, _| {
lsp_store.update_last_formatting_failure(&result);
})?;
let transaction_response = result?;
buffer_store
.update(&mut cx, |buffer_store, cx| {
buffer_store.deserialize_project_transaction(response, push_to_history, cx)
buffer_store.deserialize_project_transaction(
transaction_response,
push_to_history,
cx,
)
})?
.await
})
@ -7366,6 +7367,18 @@ impl LspStore {
lsp_action,
})
}
fn update_last_formatting_failure<T>(&mut self, formatting_result: &anyhow::Result<T>) {
match &formatting_result {
Ok(_) => self.last_formatting_failure = None,
Err(error) => {
let error_string = format!("{error:#}");
log::error!("Formatting failed: {error_string}");
self.last_formatting_failure
.replace(error_string.lines().join(" "));
}
}
}
}
impl EventEmitter<LspStoreEvent> for LspStore {}

View file

@ -104,7 +104,19 @@ impl ErrorExt for anyhow::Error {
if let Some(rpc_error) = self.downcast_ref::<RpcError>() {
rpc_error.to_proto()
} else {
ErrorCode::Internal.message(format!("{}", self)).to_proto()
ErrorCode::Internal
.message(
format!("{self:#}")
.lines()
.fold(String::new(), |mut message, line| {
if !message.is_empty() {
message.push(' ');
}
message.push_str(line);
message
}),
)
.to_proto()
}
}

View file

@ -2017,77 +2017,97 @@ impl ChannelClient {
mut incoming_rx: mpsc::UnboundedReceiver<Envelope>,
cx: &AsyncAppContext,
) -> Task<Result<()>> {
cx.spawn(|cx| {
async move {
let peer_id = PeerId { owner_id: 0, id: 0 };
while let Some(incoming) = incoming_rx.next().await {
let Some(this) = this.upgrade() else {
return anyhow::Ok(());
};
if let Some(ack_id) = incoming.ack_id {
let mut buffer = this.buffer.lock();
while buffer.front().is_some_and(|msg| msg.id <= ack_id) {
buffer.pop_front();
cx.spawn(|cx| async move {
let peer_id = PeerId { owner_id: 0, id: 0 };
while let Some(incoming) = incoming_rx.next().await {
let Some(this) = this.upgrade() else {
return anyhow::Ok(());
};
if let Some(ack_id) = incoming.ack_id {
let mut buffer = this.buffer.lock();
while buffer.front().is_some_and(|msg| msg.id <= ack_id) {
buffer.pop_front();
}
}
if let Some(proto::envelope::Payload::FlushBufferedMessages(_)) = &incoming.payload
{
log::debug!(
"{}:ssh message received. name:FlushBufferedMessages",
this.name
);
{
let buffer = this.buffer.lock();
for envelope in buffer.iter() {
this.outgoing_tx
.lock()
.unbounded_send(envelope.clone())
.ok();
}
}
if let Some(proto::envelope::Payload::FlushBufferedMessages(_)) =
&incoming.payload
{
log::debug!("{}:ssh message received. name:FlushBufferedMessages", this.name);
{
let buffer = this.buffer.lock();
for envelope in buffer.iter() {
this.outgoing_tx.lock().unbounded_send(envelope.clone()).ok();
}
let mut envelope = proto::Ack {}.into_envelope(0, Some(incoming.id), None);
envelope.id = this.next_message_id.fetch_add(1, SeqCst);
this.outgoing_tx.lock().unbounded_send(envelope).ok();
continue;
}
this.max_received.store(incoming.id, SeqCst);
if let Some(request_id) = incoming.responding_to {
let request_id = MessageId(request_id);
let sender = this.response_channels.lock().remove(&request_id);
if let Some(sender) = sender {
let (tx, rx) = oneshot::channel();
if incoming.payload.is_some() {
sender.send((incoming, tx)).ok();
}
let mut envelope = proto::Ack{}.into_envelope(0, Some(incoming.id), None);
envelope.id = this.next_message_id.fetch_add(1, SeqCst);
this.outgoing_tx.lock().unbounded_send(envelope).ok();
continue;
rx.await.ok();
}
this.max_received.store(incoming.id, SeqCst);
if let Some(request_id) = incoming.responding_to {
let request_id = MessageId(request_id);
let sender = this.response_channels.lock().remove(&request_id);
if let Some(sender) = sender {
let (tx, rx) = oneshot::channel();
if incoming.payload.is_some() {
sender.send((incoming, tx)).ok();
}
rx.await.ok();
}
} else if let Some(envelope) =
build_typed_envelope(peer_id, Instant::now(), incoming)
{
let type_name = envelope.payload_type_name();
if let Some(future) = ProtoMessageHandlerSet::handle_message(
&this.message_handlers,
envelope,
this.clone().into(),
cx.clone(),
) {
log::debug!("{}:ssh message received. name:{type_name}", this.name);
cx.foreground_executor().spawn(async move {
} else if let Some(envelope) =
build_typed_envelope(peer_id, Instant::now(), incoming)
{
let type_name = envelope.payload_type_name();
if let Some(future) = ProtoMessageHandlerSet::handle_message(
&this.message_handlers,
envelope,
this.clone().into(),
cx.clone(),
) {
log::debug!("{}:ssh message received. name:{type_name}", this.name);
cx.foreground_executor()
.spawn(async move {
match future.await {
Ok(_) => {
log::debug!("{}:ssh message handled. name:{type_name}", this.name);
log::debug!(
"{}:ssh message handled. name:{type_name}",
this.name
);
}
Err(error) => {
log::error!(
"{}:error handling message. type:{type_name}, error:{error}", this.name,
"{}:error handling message. type:{}, error:{}",
this.name,
type_name,
format!("{error:#}").lines().fold(
String::new(),
|mut message, line| {
if !message.is_empty() {
message.push(' ');
}
message.push_str(line);
message
}
)
);
}
}
}).detach()
} else {
log::error!("{}:unhandled ssh message name:{type_name}", this.name);
}
})
.detach()
} else {
log::error!("{}:unhandled ssh message name:{type_name}", this.name);
}
}
anyhow::Ok(())
}
anyhow::Ok(())
})
}