From aba2977a5a7bbc47e347e2eaa668c56a0d2f18c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 24 May 2022 12:52:18 +0200 Subject: [PATCH 1/2] ref: Log correct AppCenter error message --- src/utils/appcenter.rs | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/utils/appcenter.rs b/src/utils/appcenter.rs index c0f721708c..d8f6986e4b 100644 --- a/src/utils/appcenter.rs +++ b/src/utils/appcenter.rs @@ -8,9 +8,9 @@ use std::str; use anyhow::{bail, format_err, Error, Result}; use console::strip_ansi_codes; use glob::{glob_with, MatchOptions}; -// use serde::de::{Deserialize, Deserializer, Error as DeError}; use if_chain::if_chain; use serde::de; +use serde::Deserialize; use crate::utils::releases::{get_xcode_release_name, infer_gradle_release_name}; use crate::utils::xcode::{InfoPlist, XcodeProjectInfo}; @@ -34,6 +34,12 @@ pub struct AppCenterPackage { pub label: String, } +#[derive(Debug, Deserialize)] +pub struct AppCenterOutput { + #[serde(rename = "errorMessage")] + pub error_message: String, +} + impl<'de> de::Deserialize<'de> for AppCenterPackage { fn deserialize>(deserializer: D) -> Result { struct PackageVisitor; @@ -66,16 +72,29 @@ impl<'de> de::Deserialize<'de> for AppCenterPackage { } } +// AppCenter CLI can throw errors in 2 different formats, based on the `--output` flag, +// and we want to handle them both (we call it with `--output json` ourselves). +// +// JSON: `{"succeeded":false,"errorCode":5,"errorMessage":"Command 'appcenter codepush deployment history' requires a logged in user. Use the 'appcenter login' command to log in."}` +// Text: `Error: Command 'appcenter codepush deployment history' requires a logged in user. Use the 'appcenter login' command to log in.` +// +// Also, starting version 2.10.8 (2022-01-10), it prints to `stderr`, where it used to use `stdout` before. +// ref: https://github.com/microsoft/appcenter-cli/commit/b3d6290afcb84affe6a4096893b1ea11d10ac3cf pub fn get_appcenter_error(output: &Output) -> Error { - let message = str::from_utf8(&output.stdout).unwrap_or("Unknown AppCenter error"); - - let stripped = strip_ansi_codes(message); - let cause = if let Some(rest) = stripped.strip_prefix("Error: ") { - rest - } else { - &stripped - } - .to_string(); + let cause = serde_json::from_slice::(&output.stderr) + .map(|o| o.error_message) + .unwrap_or_else(|_| { + str::from_utf8(&output.stderr) + .map(|o| { + let stripped = strip_ansi_codes(o); + if let Some(rest) = stripped.strip_prefix("Error: ") { + rest.to_string() + } else { + stripped.to_string() + } + }) + .unwrap_or_else(|_| "Unknown AppCenter error".to_string()) + }); format_err!(cause) } From 9ed80915c17af2b6b9fae90ba27b70be467876e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 24 May 2022 13:08:51 +0200 Subject: [PATCH 2/2] ref: Change comment format for find_matching_artifact --- src/commands/sourcemaps/explain.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/commands/sourcemaps/explain.rs b/src/commands/sourcemaps/explain.rs index 71038856e2..52e4afb5e6 100644 --- a/src/commands/sourcemaps/explain.rs +++ b/src/commands/sourcemaps/explain.rs @@ -113,20 +113,18 @@ fn fetch_release_artifacts(org: &str, project: &str, release: &str) -> Result ~/dist/bundle.min.js - * /dist/bundle.js.map => ~/dist/bundle.js.map - * okboomer => error (invalid relative path, no extension) - * - * It should be more generic than using the defaults, but should be sufficient for our current usecase. - */ +// Try to find an artifact which matches the path part of the url extracted from the stacktrace frame, +// prefixed with the default `~/`, which is a "glob-like" pattern for matchin any hostname. +// +// We only need the `pathname` portion of the url, so if it's absolute, just extract it. +// If it's relative however, parse any random url (example.com) and join it with our relative url, +// as Rust cannot handle parsing of relative urls. +// +// http://localhost:5000/dist/bundle.min.js => ~/dist/bundle.min.js +// /dist/bundle.js.map => ~/dist/bundle.js.map +// okboomer => error (invalid relative path, no extension) +// +// It should be more generic than using the defaults, but should be sufficient for our current usecase. fn find_matching_artifact(artifacts: &[Artifact], abs_path: &str) -> Result { let abs_path = match Url::parse(abs_path) { Ok(path) => Ok(path),