Browse Source

Report API plugin load errors to the user

fork/Issue_3453_extruded_3d_body
Jon Evans 3 months ago
parent
commit
c81ed537c3
  1. 55
      common/api/api_plugin.cpp
  2. 99
      common/api/api_plugin_manager.cpp
  3. 22
      common/tool/common_control.cpp
  4. 11
      eeschema/tools/sch_editor_control.cpp
  5. 1
      eeschema/tools/sch_editor_control.h
  6. 8
      include/api/api_plugin.h
  7. 7
      include/api/api_plugin_manager.h
  8. 1
      include/tool/common_control.h
  9. 21
      pcbnew/dialogs/panel_pcbnew_action_plugins.cpp
  10. 3
      pcbnew/dialogs/panel_pcbnew_action_plugins.h

55
common/api/api_plugin.cpp

@ -46,24 +46,31 @@ void LOGGING_ERROR_HANDLER::error( const nlohmann::json::json_pointer& ptr,
wxLogTrace( traceApi,
wxString::Format( wxS( "JSON error: at %s, value:\n%s\n%s" ),
ptr.to_string(), instance.dump(), message ) );
wxString location = wxString::FromUTF8( ptr.to_string() );
if( location.IsEmpty() )
location = wxS( "/" );
if( !m_errorMessage.IsEmpty() )
m_errorMessage << '\n';
m_errorMessage << wxString::Format( _( "invalid plugin configuration at '%s': %s" ),
location, wxString::FromUTF8( message ) );
}
bool PLUGIN_RUNTIME::FromJson( const nlohmann::json& aJson )
tl::expected<bool, wxString> PLUGIN_RUNTIME::FromJson( const nlohmann::json& aJson )
{
// TODO move to tl::expected and give user feedback about parse errors
try
{
type = magic_enum::enum_cast<PLUGIN_RUNTIME_TYPE>( aJson.at( "type" ).get<std::string>(),
magic_enum::case_insensitive )
.value_or( PLUGIN_RUNTIME_TYPE::INVALID );
}
catch( ... )
catch( std::exception& e )
{
return false;
return tl::unexpected( wxString::Format( _( "invalid plugin runtime: %s" ), e.what() ) );
}
return type != PLUGIN_RUNTIME_TYPE::INVALID;
@ -76,6 +83,7 @@ struct API_PLUGIN_CONFIG
const JSON_SCHEMA_VALIDATOR& aValidator );
bool valid;
wxString error_message;
wxString identifier;
wxString name;
wxString description;
@ -93,7 +101,10 @@ API_PLUGIN_CONFIG::API_PLUGIN_CONFIG( API_PLUGIN& aParent, const wxFileName& aCo
valid = false;
if( !aConfigFile.IsFileReadable() )
{
error_message = _( "could not read plugin configuration file" );
return;
}
wxLogTrace( traceApi, "Plugin: parsing config file" );
@ -108,9 +119,11 @@ API_PLUGIN_CONFIG::API_PLUGIN_CONFIG( API_PLUGIN& aParent, const wxFileName& aCo
/* allow_exceptions = */ true,
/* ignore_comments = */ true );
}
catch( ... )
catch( const std::exception& e )
{
wxLogTrace( traceApi, "Plugin: exception during parse" );
error_message = wxString::Format( _( "plugin configuration file error: %s" ),
wxString::FromUTF8( e.what() ) );
return;
}
@ -119,6 +132,8 @@ API_PLUGIN_CONFIG::API_PLUGIN_CONFIG( API_PLUGIN& aParent, const wxFileName& aCo
if( !handler.HasError() )
wxLogTrace( traceApi, "Plugin: schema validation successful" );
else
error_message = handler.ErrorMessage();
// All of these are required; any exceptions here leave us with valid == false
try
@ -127,15 +142,21 @@ API_PLUGIN_CONFIG::API_PLUGIN_CONFIG( API_PLUGIN& aParent, const wxFileName& aCo
name = js.at( "name" ).get<wxString>();
description = js.at( "description" ).get<wxString>();
if( !runtime.FromJson( js.at( "runtime" ) ) )
if( !runtime.FromJson( js.at( "runtime" ) ).or_else(
[this]( const wxString& aError )
{
wxLogTrace( traceApi, "Plugin %s: %s", identifier, aError );
error_message = aError;
} ).has_value() )
{
wxLogTrace( traceApi, "Plugin: error parsing runtime section" );
return;
}
}
catch( ... )
catch( const std::exception& e )
{
wxLogTrace( traceApi, "Plugin: exception while parsing required keys" );
error_message = wxString::Format( _( "missing or invalid required keys: %s" ),
wxString::FromUTF8( e.what() ) );
return;
}
@ -143,6 +164,7 @@ API_PLUGIN_CONFIG::API_PLUGIN_CONFIG( API_PLUGIN& aParent, const wxFileName& aCo
{
wxLogTrace( traceApi, wxString::Format( "Plugin: identifier %s does not meet requirements",
identifier ) );
error_message = wxString::Format( _( "identifier '%s' is invalid" ), identifier );
return;
}
@ -166,13 +188,16 @@ API_PLUGIN_CONFIG::API_PLUGIN_CONFIG( API_PLUGIN& aParent, const wxFileName& aCo
}
}
}
catch( ... )
catch( const std::exception& e )
{
wxLogTrace( traceApi, "Plugin: exception while parsing actions" );
error_message = wxString::Format( _( "actions section is invalid: %s" ),
wxString::FromUTF8( e.what() ) );
return;
}
valid = true;
error_message = wxEmptyString;
}
@ -194,6 +219,12 @@ bool API_PLUGIN::IsOk() const
}
const wxString& API_PLUGIN::ErrorMessage() const
{
return m_config->error_message;
}
bool API_PLUGIN::IsValidIdentifier( const wxString& aIdentifier )
{
// At minimum, we need a reverse-DNS style identifier with two dots and a 2+ character TLD

99
common/api/api_plugin_manager.cpp

@ -100,6 +100,17 @@ static void reportPluginActionMessage( REPORTER* aReporter, const wxString& aAct
}
static void reportPluginLoadMessage( REPORTER* aReporter, const wxString& aPluginName,
const wxString& aMessage )
{
if( !aReporter || aMessage.IsEmpty() )
return;
aReporter->Report( wxString::Format( _( "Plugin '%s': %s" ), aPluginName, aMessage ),
RPT_SEVERITY_ERROR );
}
static void reportPluginActionResult( REPORTER* aReporter, const wxString& aActionName,
int aRetVal, const wxString& aError )
{
@ -163,8 +174,11 @@ public:
};
void API_PLUGIN_MANAGER::ReloadPlugins( std::optional<wxString> aDirectoryToScan )
void API_PLUGIN_MANAGER::ReloadPlugins( std::optional<wxString> aDirectoryToScan,
std::shared_ptr<REPORTER> aReporter )
{
m_reloadReporter = std::move( aReporter );
m_plugins.clear();
m_pluginsCache.clear();
m_actionsCache.clear();
@ -183,17 +197,20 @@ void API_PLUGIN_MANAGER::ReloadPlugins( std::optional<wxString> aDirectoryToScan
if( plugin->IsOk() )
{
if( m_pluginsCache.count( plugin->Identifier() ) )
const wxString& id = plugin->Identifier();
if( m_pluginsCache.contains( id ) )
{
wxLogTrace( traceApi,
wxString::Format( "Manager: identifier %s already present!",
plugin->Identifier() ) );
wxLogTrace( traceApi, wxString::Format( "Manager: identifier %s already present, reloading", id ) );
for( const PLUGIN_ACTION& action : m_pluginsCache[id]->Actions() )
m_actionsCache[action.identifier] = &action;
m_pluginsCache.erase( id );
return;
}
else
{
m_pluginsCache[plugin->Identifier()] = plugin.get();
}
m_pluginsCache[id] = plugin.get();
for( const PLUGIN_ACTION& action : plugin->Actions() )
m_actionsCache[action.identifier] = &action;
@ -203,6 +220,9 @@ void API_PLUGIN_MANAGER::ReloadPlugins( std::optional<wxString> aDirectoryToScan
else
{
wxLogTrace( traceApi, "Manager: loading failed" );
reportPluginLoadMessage( m_reloadReporter.get(), aFile.GetFullPath(),
plugin->ErrorMessage() );
}
} );
@ -253,6 +273,9 @@ void API_PLUGIN_MANAGER::ReloadPlugins( std::optional<wxString> aDirectoryToScan
processPluginDependencies();
if( !Busy() )
m_reloadReporter.reset();
wxCommandEvent* evt = new wxCommandEvent( EDA_EVT_PLUGIN_AVAILABILITY_CHANGED, wxID_ANY );
m_parent->QueueEvent( evt );
}
@ -343,7 +366,7 @@ int API_PLUGIN_MANAGER::doInvokeAction( const wxString& aIdentifier, std::vector
wxLogTrace( traceApi, wxString::Format( "Manager: Python interpreter for %s not found",
plugin.Identifier() ) );
reportPluginActionMessage( aReporter.get(), action->name,
_( "Python environment does not exist" ) );
_( "missing plugin environment" ) );
return -1;
}
@ -709,7 +732,7 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
};
manager.Execute( args,
[this]( int aRetVal, const wxString& aOutput, const wxString& aError )
[this, job]( int aRetVal, const wxString& aOutput, const wxString& aError )
{
wxLogTrace( traceApi,
wxString::Format( "Manager: created venv (python returned %d)", aRetVal ) );
@ -717,6 +740,19 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
if( !aError.IsEmpty() )
wxLogTrace( traceApi, wxString::Format( "Manager: venv err: %s", aError ) );
if( aRetVal != 0 )
{
wxString error = aError;
error.Trim().Trim( false );
if( error.IsEmpty() )
error = wxString::Format( _( "error code %d" ), aRetVal );
error = wxString::Format( _( "could not create plugin environment: %s" ), error );
reportPluginLoadMessage( m_reloadReporter.get(), job.identifier, error );
}
wxCommandEvent* evt =
new wxCommandEvent( EDA_EVT_PLUGIN_MANAGER_JOB_FINISHED, wxID_ANY );
QueueEvent( evt );
@ -738,6 +774,9 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
{
wxLogTrace( traceApi, wxString::Format( "Manager: error: python not found at %s",
job.env_path ) );
reportPluginLoadMessage( m_reloadReporter.get(), job.identifier,
_( "missing plugin environment" ) );
}
else
{
@ -772,7 +811,7 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
};
manager.Execute( args,
[this]( int aRetVal, const wxString& aOutput, const wxString& aError )
[this, job]( int aRetVal, const wxString& aOutput, const wxString& aError )
{
wxLogTrace( traceApi, wxString::Format( "Manager: upgrade pip returned %d",
aRetVal ) );
@ -783,6 +822,19 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
wxString::Format( "Manager: upgrade pip stderr: %s", aError ) );
}
if( aRetVal != 0 )
{
wxString error = aError;
error.Trim().Trim( false );
if( error.IsEmpty() )
error = wxString::Format( _( "error code %d" ), aRetVal );
error = wxString::Format( _( "could not create plugin environment: %s" ), error );
reportPluginLoadMessage( m_reloadReporter.get(), job.identifier, error );
}
wxCommandEvent* evt =
new wxCommandEvent( EDA_EVT_PLUGIN_MANAGER_JOB_FINISHED, wxID_ANY );
QueueEvent( evt );
@ -806,12 +858,18 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
{
wxLogTrace( traceApi, wxString::Format( "Manager: error: python not found at %s",
job.env_path ) );
reportPluginLoadMessage( m_reloadReporter.get(), job.identifier,
_( "missing plugin environment" ) );
}
else if( !reqs.IsFileReadable() )
{
wxLogTrace( traceApi,
wxString::Format( "Manager: error: requirements.txt not found at %s",
job.plugin_path ) );
reportPluginLoadMessage( m_reloadReporter.get(), job.identifier,
_( "requirements.txt could not be read" ) );
}
else
{
@ -854,6 +912,19 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
if( !aError.IsEmpty() )
wxLogTrace( traceApi, wxString::Format( "Manager: pip stderr: %s", aError ) );
if( aRetVal != 0 )
{
wxString error = aError;
error.Trim().Trim( false );
if( error.IsEmpty() )
error = wxString::Format( _( "error code %d" ), aRetVal );
error = wxString::Format( _( "could not create plugin environment: %s" ), error );
reportPluginLoadMessage( m_reloadReporter.get(), job.identifier, error );
}
if( aRetVal == 0 )
{
wxLogTrace( traceApi, wxString::Format( "Manager: marking %s as ready",
@ -879,6 +950,10 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
}
m_jobs.pop_front();
if( !Busy() )
m_reloadReporter.reset();
wxLogTrace( traceApi, wxString::Format( "Manager: finished job; %zu left in queue",
m_jobs.size() ) );
}

22
common/tool/common_control.cpp

@ -37,6 +37,8 @@
#include <gal/graphics_abstraction_layer.h>
#include <base_screen.h>
#include <tool/common_control.h>
#include <api/api_plugin_manager.h>
#include <id.h>
#include <kiface_base.h>
#include <dialogs/dialog_configure_paths.h>
@ -46,6 +48,8 @@
#include <gestfich.h>
#include <tools/kicad_manager_actions.h>
#include <confirm.h>
#include <reporter.h>
#include <widgets/kistatusbar.h>
#define URL_GET_INVOLVED wxS( "https://go.kicad.org/contribute/" )
#define URL_DONATE wxS( "https://go.kicad.org/app-donate" )
@ -425,6 +429,23 @@ int COMMON_CONTROL::ReportBug( const TOOL_EVENT& aEvent )
}
int COMMON_CONTROL::ReloadPlugins( const TOOL_EVENT& aEvent )
{
#ifdef KICAD_IPC_API
if( Pgm().GetCommonSettings()->m_Api.enable_server )
{
std::shared_ptr<REPORTER> reporter;
if( KISTATUSBAR* statusBar = dynamic_cast<KISTATUSBAR*>( m_frame->GetStatusBar() ) )
reporter = std::make_shared<STATUSBAR_WARNING_REPORTER>( statusBar, wxS( "plugin" ) );
Pgm().GetPluginManager().ReloadPlugins( std::nullopt, reporter );
}
#endif
return 0;
}
void COMMON_CONTROL::setTransitions()
{
Go( &COMMON_CONTROL::Quit, ACTIONS::quit.MakeEvent() );
@ -448,6 +469,7 @@ void COMMON_CONTROL::setTransitions()
Go( &COMMON_CONTROL::Donate, ACTIONS::donate.MakeEvent() );
Go( &COMMON_CONTROL::ReportBug, ACTIONS::reportBug.MakeEvent() );
Go( &COMMON_CONTROL::About, ACTIONS::about.MakeEvent() );
Go( &COMMON_CONTROL::ReloadPlugins, ACTIONS::pluginsReload.MakeEvent() );
}

11
eeschema/tools/sch_editor_control.cpp

@ -3316,15 +3316,6 @@ int SCH_EDITOR_CONTROL::ToggleAnnotateAuto( const TOOL_EVENT& aEvent )
}
int SCH_EDITOR_CONTROL::ReloadPlugins( const TOOL_EVENT& aEvent )
{
#ifdef KICAD_IPC_API
if( Pgm().GetCommonSettings()->m_Api.enable_server )
Pgm().GetPluginManager().ReloadPlugins();
#endif
return 0;
}
int SCH_EDITOR_CONTROL::OnAngleSnapModeChanged( const TOOL_EVENT& aEvent )
{
// Update the left toolbar Line modes group icon to match current mode
@ -3573,8 +3564,6 @@ void SCH_EDITOR_CONTROL::setTransitions()
Go( &SCH_EDITOR_CONTROL::OnAngleSnapModeChanged, SCH_ACTIONS::angleSnapModeChanged.MakeEvent() );
Go( &SCH_EDITOR_CONTROL::ToggleAnnotateAuto, SCH_ACTIONS::toggleAnnotateAuto.MakeEvent() );
Go( &SCH_EDITOR_CONTROL::ReloadPlugins, ACTIONS::pluginsReload.MakeEvent() );
Go( &SCH_EDITOR_CONTROL::ExportSymbolsToLibrary, SCH_ACTIONS::exportSymbolsToLibrary.MakeEvent() );
Go( &SCH_EDITOR_CONTROL::PlaceLinkedDesignBlock, SCH_ACTIONS::placeLinkedDesignBlock.MakeEvent() );

1
eeschema/tools/sch_editor_control.h

@ -149,7 +149,6 @@ public:
int NextLineMode( const TOOL_EVENT& aEvent );
int OnAngleSnapModeChanged( const TOOL_EVENT& aEvent );
int ToggleAnnotateAuto( const TOOL_EVENT& aEvent );
int ReloadPlugins( const TOOL_EVENT& aEvent );
int GridFeedback( const TOOL_EVENT& aEvent );

8
include/api/api_plugin.h

@ -26,6 +26,7 @@
#include <set>
#include <nlohmann/json_fwd.hpp>
#include <nlohmann/json-schema.hpp>
#include <tl/expected.hpp>
#include <wx/bmpbndl.h>
#include <wx/filename.h>
#include <wx/string.h>
@ -56,7 +57,7 @@ enum class PLUGIN_RUNTIME_TYPE
struct PLUGIN_RUNTIME
{
bool FromJson( const nlohmann::json& aJson );
tl::expected<bool, wxString> FromJson( const nlohmann::json& aJson );
PLUGIN_RUNTIME_TYPE type;
wxString min_version;
@ -100,6 +101,8 @@ public:
bool IsOk() const;
const wxString& ErrorMessage() const;
static bool IsValidIdentifier( const wxString& aIdentifier );
const wxString& Identifier() const;
@ -142,11 +145,14 @@ public:
bool HasError() const { return m_hasError; }
const wxString& ErrorMessage() const { return m_errorMessage; }
void error( const nlohmann::json::json_pointer& ptr, const nlohmann::json& instance,
const std::string& message ) override;
private:
bool m_hasError;
wxString m_errorMessage;
};
#endif //KICAD_API_PLUGIN_H

7
include/api/api_plugin_manager.h

@ -51,7 +51,8 @@ public:
* @param aDirectoryToScan can be provided to scan an arbitrary directory instead of the
* stock paths; provided for QA testing.
*/
void ReloadPlugins( std::optional<wxString> aDirectoryToScan = std::nullopt );
void ReloadPlugins( std::optional<wxString> aDirectoryToScan = std::nullopt,
std::shared_ptr<REPORTER> aReporter = nullptr );
void RecreatePluginEnvironment( const wxString& aIdentifier );
@ -82,6 +83,8 @@ public:
std::map<int, wxString>& MenuBindings() { return m_menuBindings; }
std::shared_ptr<REPORTER> GetReporter() { return m_reloadReporter; }
private:
void processPluginDependencies();
@ -132,6 +135,8 @@ private:
std::unique_ptr<JSON_SCHEMA_VALIDATOR> m_schema_validator;
std::shared_ptr<REPORTER> m_reloadReporter;
[[maybe_unused]] long m_lastPid;
[[maybe_unused]] wxTimer* m_raiseTimer;
};

1
include/tool/common_control.h

@ -60,6 +60,7 @@ public:
int GetInvolved( const TOOL_EVENT& aEvent );
int Donate( const TOOL_EVENT& aEvent );
int ReportBug( const TOOL_EVENT& aEvent );
int ReloadPlugins( const TOOL_EVENT& aEvent );
///< Sets up handlers for various events.
void setTransitions() override;

21
pcbnew/dialogs/panel_pcbnew_action_plugins.cpp

@ -29,13 +29,17 @@
#include <pcb_edit_frame.h>
#include <pcbnew_settings.h>
#include <pgm_base.h>
#include <reporter.h>
#include <settings/common_settings.h>
#include <api/api_plugin_manager.h>
#include <dialog_HTML_reporter_base.h>
#include <launch_ext.h>
#include <widgets/kistatusbar.h>
#include <widgets/grid_icon_text_helpers.h>
#include <widgets/paged_dialog.h>
#include <widgets/wx_grid.h>
#include <widgets/std_bitmap_button.h>
#include <widgets/wx_html_report_box.h>
#include <wx/app.h>
@ -123,6 +127,9 @@ PANEL_PCBNEW_ACTION_PLUGINS::PANEL_PCBNEW_ACTION_PLUGINS( wxWindow* aParent ) :
m_reloadButton->SetBitmap( KiBitmapBundle( BITMAPS::small_refresh ) );
m_showErrorsButton->SetBitmap( KiBitmapBundle( BITMAPS::small_warning ) );
m_errorDialog = new DIALOG_HTML_REPORTER( aParent );
m_allowErrorDialog = false;
wxTheApp->Bind( EDA_EVT_PLUGIN_AVAILABILITY_CHANGED,
&PANEL_PCBNEW_ACTION_PLUGINS::onPluginAvailabilityChanged, this );
}
@ -130,6 +137,7 @@ PANEL_PCBNEW_ACTION_PLUGINS::PANEL_PCBNEW_ACTION_PLUGINS( wxWindow* aParent ) :
PANEL_PCBNEW_ACTION_PLUGINS::~PANEL_PCBNEW_ACTION_PLUGINS()
{
delete m_errorDialog;
wxTheApp->Unbind( EDA_EVT_PLUGIN_AVAILABILITY_CHANGED,
&PANEL_PCBNEW_ACTION_PLUGINS::onPluginAvailabilityChanged, this );
m_grid->PopEventHandler( true );
@ -140,6 +148,14 @@ void PANEL_PCBNEW_ACTION_PLUGINS::onPluginAvailabilityChanged( wxCommandEvent& a
{
m_grid->Enable();
TransferDataToWindow();
if( m_allowErrorDialog && m_errorDialog->m_Reporter->HasMessage() )
{
m_errorDialog->m_Reporter->Flush();
m_allowErrorDialog = false;
m_errorDialog->ShowModal();
}
aEvt.Skip();
}
@ -195,7 +211,10 @@ void PANEL_PCBNEW_ACTION_PLUGINS::SwapRows( int aRowA, int aRowB )
void PANEL_PCBNEW_ACTION_PLUGINS::OnReloadButtonClick( wxCommandEvent& event )
{
API_PLUGIN_MANAGER& mgr = Pgm().GetPluginManager();
mgr.ReloadPlugins();
m_errorDialog->m_Reporter->Clear();
auto reporter = std::make_shared<REDIRECT_REPORTER>( m_errorDialog->m_Reporter );
m_allowErrorDialog = true;
mgr.ReloadPlugins( std::nullopt, reporter );
m_grid->Disable();
}

3
pcbnew/dialogs/panel_pcbnew_action_plugins.h

@ -21,6 +21,7 @@
#include "panel_pcbnew_action_plugins_base.h"
class PLUGINS_GRID_TRICKS;
class DIALOG_HTML_REPORTER;
class PANEL_PCBNEW_ACTION_PLUGINS : public PANEL_PCBNEW_ACTION_PLUGINS_BASE
@ -76,6 +77,8 @@ private:
};
wxBitmapBundle m_genericIcon;
DIALOG_HTML_REPORTER* m_errorDialog;
bool m_allowErrorDialog;
void SwapRows( int aRowA, int aRowB );
void SelectRow( int aRow );

Loading…
Cancel
Save