Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sentry integration with hosted WPF application #1872

Closed
andher1802 opened this issue Aug 22, 2022 · 20 comments
Closed

Sentry integration with hosted WPF application #1872

andher1802 opened this issue Aug 22, 2022 · 20 comments
Labels
Question Further information is requested

Comments

@andher1802
Copy link

andher1802 commented Aug 22, 2022

Environment

SaaS (https://sentry.io/)

Steps to Reproduce

Hello,

Currently I am developing an ArcGIS Pro (v.2.9) AddIn using the .net SDK (WPF Application) that I would like to integrate my Sentry account for error tracking. I have been trying to configure the Sentry account and everything seems to be OK, but the errors and messages are not being sent back.

I tried with a non-ArgGIS test application and it works. After some debugging, I found that the object that resulted from the configuration of the Sentry.Init has a property named isEnabled that is true on my test application, but it is false in the ArcGIS AddIn. (See debug picture).

My question is: Is the integration with User Error tracking allowed within ArcGIS AddIns? And if so, How do I configure my project to allow this.

Many thanks.

Actual Result

ArcGISDebugScreenshot

@getsentry-release
Copy link
Collaborator

Routing to @getsentry/team-web-sdk-backend for triage. ⏲️

@getsentry-release
Copy link
Collaborator

Routing to @getsentry/team-mobile for triage. ⏲️

@mattjohnsonpint mattjohnsonpint transferred this issue from getsentry/sentry Aug 23, 2022
@mattjohnsonpint mattjohnsonpint added Question Further information is requested Platform: .NET and removed Team: Mobile Platform labels Aug 23, 2022
@mattjohnsonpint
Copy link
Contributor

Hi. Instead of screenshots, could you please provide some example code of how you initialize Sentry in the application? I know very little about ArcGIS other than what I can google.

My guess is that the DSN is either not set, or the Init function hasn't executed. It could also be that you've already disposed the result of the init.

@andher1802
Copy link
Author

andher1802 commented Aug 23, 2022

Hi. Instead of screenshots, could you please provide some example code of how you initialize Sentry in the application? I know very little about ArcGIS other than what I can google.

My guess is that the DSN is either not set, or the Init function hasn't executed. It could also be that you've already disposed the result of the init.

Many thanks for your reply,

Below you can find the class where I initialized the sentry configuration (BaseVM constructor). I did it the same way as in the tipical WPF application.

The difference is that in a regular WPF, I do it in the App.xaml.cs, but in ArcGIS there is not such entry point.

Therefore, I used the Application.current.DispatcherUnhandledException. However, even though I am sure my code excecutes all the lines related to sending messages to sentry, it just goes through it but does not capture anything (it does not show any error message).

The only thing I realized while debugging is this isEnabled flag, in ArcGIS is set to False at the point of message capturing while in the Typical WPF is set to true.

Best regards,

Andres H.

using ArcGIS.Core.Geometry;
using Sentry;
using System;
using System.ComponentModel;
using System.Diagnostics;
using System.Windows;
using System.Windows.Threading;
using UP42AddIn.View;
using static UP42NET.Model.UP42Auth;

namespace UP42AddIn.ViewModel
{
    public class BaseVM: INotifyPropertyChanged
    {
        private IDisposable sentryObj;

        public IDisposable SentryObj
        {
            get { return sentryObj; }
            set { 
                sentryObj = value;
                OnPropertyChanged("SentryObj");
            }
        }
        public BaseVM() 
        {
            SentryObj = SentrySdk.Init(o =>
            {
                o.Dsn = "my DNS url";
                o.Debug = true;
                o.TracesSampleRate = 1;
            });
            Application.Current.DispatcherUnhandledException += App_DispatcherUnhandledException;
        }

        void App_DispatcherUnhandledException(object sender, DispatcherUnhandledExceptionEventArgs e)
        {
            SentrySdk.CaptureException(e.Exception);
            e.Handled = true;
        }

@mattjohnsonpint
Copy link
Contributor

Thanks. Some more questions:

  • What is BaseVM? Is that an ArcGIS specific thing, or some other base class of your own application? How does it get invoked?
  • I see you are saving SentryObj. Are you disposing it somewhere?

Typically we would want to initialize Sentry as early as possible in the application's lifecycle - not on a particular control or view.

@andher1802
Copy link
Author

Thanks. Some more questions:

  • What is BaseVM? Is that an ArcGIS specific thing, or some other base class of your own application? How does it get invoked?
  • I see you are saving SentryObj. Are you disposing it somewhere?

Typically we would want to initialize Sentry as early as possible in the application's lifecycle - not on a particular control or view.

Thanks again, I really appreciate your help:

  1. BaseVM is just a class I used that is invoked when I click the button for make the AddIn appear. An ArcGIS AddIn is a plugin we used for extending ArcGIS (Think in an Excel AddIn). Therefore, the earliest point where I can call Sentry is where the AddIn is called (When the BaseVM is invoked). I am not sure if maybe there is a limitation from ArcGIS that can prevent Sentry to capture errors, but I would like to know what is this isEnabled flag, and if there is some way to manually set it to true.

  2. I do not dispose SentryObj anywhere.

@mattjohnsonpint
Copy link
Contributor

IsEnabled is set true automatically when you initialize the SDK with a valid DSN. It's set false if the SDK is attempted to be initialized without a valid DSN, or when the return from init is disposed. It's not something you set directly.

I see you have Debug = true. Do you have any log output to share?

@mattjohnsonpint
Copy link
Contributor

Also, you should set IsGlobalModeEnabled = true on the options.

@andher1802
Copy link
Author

IsGlobalModeEnabled = true

Thanks, I tried with no luck. Maybe the point where I can init sentry is not allowing to configure it properly, but I can't add other entry point more than what ArcGIS allows me. Debug log outputs are not showing anything, it is like it is working but without sending any message to the sentry server.

@mattjohnsonpint
Copy link
Contributor

mattjohnsonpint commented Aug 23, 2022

Ok. Thanks for checking. You'll probably want that on anyway - we usually recommend global mode for client-based apps.

I'm out of the office this week, but perhaps @bruno-garcia or @SimonCropp can investigate further. If not, I'll dig in when I can.

If you have some getting started docs to point us at that demonstrate the type of arcgis plug in you're writing, that would help. A minimal complete example we can test with would be even better.

And just fair warning, while I don't see any direct reason this shouldn't work, Arcgis isn't specifically an environment we have officially offered support for. So if it turns out there is some kind of special runtime restriction, then we might not necessarily be able to offer a solution.

Lastly, it would be good to know what kind of barriers (if any) are established by Arcgis. For example, what would happen if two different plug-ins tried to initialize Sentry with two different DSNs? If they are in process boundaries or some other sort of sandbox, then you might be OK. But if they are sharing a process then it could be last-one-wins.

@SimonCropp
Copy link
Contributor

@andher1802 I think there are too many moving pieces here. can we get a minimal sample project that reproduces the issue so we can debug into it

@CADbloke
Copy link

CADbloke commented Aug 25, 2022

Application.Current.DispatcherUnhandledException may already me marked as handled by another event delegate. That event is basically owned by the host application, not your plugin and the host probably deals with it before it gets to your handler. Probably, I'm not certain how the invocation list is managed by that event when it is marked as handled.

I use Sentry in AutoCAD plugins, it's always in try...catch scenarios with a logger, Serilog in my case.

For example, what would happen if two different plug-ins tried to initialize Sentry with two different DSNs? If they are in process boundaries or some other sort of sandbox, then you might be OK. But if they are sharing a process then it could be last-one-wins.

That is exactly what happens, last one wins. I use a static Logger property in my plugin assembly and log to that which sends those events to Sentry. Don't hijack the host application's exception handling, particularly if you don't know what it is. Lessons learned from AutoCAD.

@andher1802
Copy link
Author

@andher1802 I think there are too many moving pieces here. can we get a minimal sample project that reproduces the issue so we can debug into it

Dear @SimonCropp, many thanks for your reply. I will create an example and share it with you.

@andher1802
Copy link
Author

andher1802 commented Aug 25, 2022

Application.Current.DispatcherUnhandledException may already me marked as handled by another event delegate. That event is basically owned by the host application, not your plugin and the host probably deals with it before it gets to your handler. Probably, I'm not certain how the invocation list is managed by that event when it is marked as handled.

I use Sentry in AutoCAD plugins, it's always in try...catch scenarios with a logger, Serilog in my case.

For example, what would happen if two different plug-ins tried to initialize Sentry with two different DSNs? If they are in process boundaries or some other sort of sandbox, then you might be OK. But if they are sharing a process then it could be last-one-wins.

That is exactly what happens, last one wins. I use a static Logger property in my plugin assembly and log to that which sends those events to Sentry. Don't hijack the host application's exception handling, particularly if you don't know what it is. Lessons learned from AutoCAD.

Dear @CADbloke, many thanks for your comments. I will take a look into it. I made sure that my error handling function is excecuted, but as you mentioned, I am not sure what is happening behind and how the host application deals with errors. I have created as well a post on the ESRI developers community and I havent got not too many clues about this. However, I think the reply I got is on the same direction you are pointing. https://community.esri.com/t5/arcgis-pro-sdk-questions/arcgis-pro-addin-integration-with-user-error/m-p/1205148#M8628 . They pointed me out to this reference, which I am currently reading https://github.com/Esri/arcgis-pro-sdk/wiki/ProConcepts-Framework#diagnostic-mode-event-logging.

@andher1802
Copy link
Author

andher1802 commented Sep 6, 2022

Hello,

Sorry for not being able to provide the minimum project for recreating the error so far. However, as it is an integration with ArcGIS Pro, you would require to have also ArcGIS Pro Installed to be able to run such projects. Nevertheless, I am still working on integrating those applications with no successful results.

However, we have made a recent discovery that could give some hints on this issue and also a potential dependency bug for Sentry. Firstly, we needed to catch the sentry logs as they were not returning anything on my WPF application log. Once we solved that issue by setting an own diagnostic logger, we got the following exception from Sentry.

System.Exception: Sentry Exception occurred ---> System.IO.FileNotFoundException: Could not load file or assembly 'System.Runtime.CompilerServices.Unsafe, Version=4.0.4.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified.

My current version of the System.Runtime.CompilerServices.Unsafe is 6.0.0 and I have not been able to downgrade due to some other dependencies.

However, I have a test dummy WPF application with no ArcGIS involved. This dummy application works when we set the System.Runtime.CompilerServices.Unsafe dependency to version 5.0.0, but it fails when we setted to 6.0.0.

Do you think there is any way to get rid of this dependency issues without having to downgrade the CompilerServices.Unsafe.

Many thanks.
P.S. I attach the WPF dummy application that is not working, please if you can take a look at the dependencies (specially to the CompilerServices.Unsafe).

SentyTestWPF.zip

@mattjohnsonpint mattjohnsonpint moved this from Needs Discussion to Needs Review in Mobile & Cross Platform SDK Sep 15, 2022
@mattjohnsonpint
Copy link
Contributor

See my comments on #1901 for dealing with System.Runtime.ComplierServices.Unsafe using binding redirection.

@mattjohnsonpint
Copy link
Contributor

mattjohnsonpint commented Sep 19, 2022

After discussing with my colleagues, we would discourage you from using Sentry in this scenario. Sentry is intended for application developers, not developers of add-ins for those applications.

Some previous discussion on this (in regard to the Android SDK, but applies universally) is here: getsentry/sentry-docs#3611

This guidance still needs to be added to our official documentation, but I agree with the reasons stated there. Decisions like whether PII can be sent to Sentry should be up to the app developer, not add-in developers (IMHO).

From a technical perspective, the Sentry .NET SDK has a static SentrySDK class that is expected to be initialized by the application for use across the entire application. While you can swap out SentryClient within a particular scope, all of the scopes are managed by the hub owned by SentrySDK. In the case where two different add-ins were to call SentrySDK.Init, the last one to initialize would get data from both add-ins. So unless the application offers a process boundary (having your add-in run in a separate process than the main application), it’s probably not a good idea.

Repository owner moved this from Needs Review to Done in Mobile & Cross Platform SDK Sep 19, 2022
@CADbloke
Copy link

CADbloke commented Oct 3, 2022

@mattjohnsonpint Darn. Is there no way around this with something like an instance (instead of static) Sentry client?

These host applications are things on the scale of Excel, Word etc. A lot of add-ins exist for these types of things, they have stores selling them etc. This limitation basically counts Sentry out as an option for any add-in developer in any application. That's a lot of exclusions for a commercial application. Think WordPress add-ins for an idea of scale.

As for the sending PII decision, that is an easy enough option to implement in an add-in. I have implemented it in all of my apps & add-ins.

Cheers
Ewen

@mattjohnsonpint
Copy link
Contributor

You could explore the approach taken in this sample where there are two different SentryClient instances (pointing at two different DSNs, one for regular requests and one for admin requests, in that example). The mechanism is via PushScope and then BindClient, both on SentrySDK (which is static).

If all the add-ins in an app were following that pattern, it should work (in theory). But since we can't guarantee such things, it's absolutely possible for one add-in to usurp another.

The only real safety would be a process boundary. BTW, modern Excel add-ins do provide a process boundary, per these docs:

  • Isolates the process the add-in runs in.

I don't believe that applies though to the older COM or VSTO style add-ins.

I'm really not sure which approach ArcGIS takes. I didn't see anything prominent in the docs, but perhaps I didn't dig deep enough.

@CADbloke
Copy link

CADbloke commented Oct 3, 2022

I'm not sure about ArcGis either. I do know that AutoCAD, BricsCAD etc are in-process, the plugin DLL is loaded in process and becomes part of the application's feature set.

So, um, yeah. Back to the drawing board.
Thanks for the info & updates, much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question Further information is requested
Projects
Archived in project
Development

No branches or pull requests

7 participants