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

Electron - For the module field, exclude anything before .asar/ #686

Closed
armenzg opened this issue Jun 13, 2023 · 13 comments · Fixed by getsentry/sentry-javascript#8488 or #699
Closed

Comments

@armenzg
Copy link
Member

armenzg commented Jun 13, 2023

I'm noticing Electron stacktraces including a full path for the module value rather than just the module.
For instance, this: C:\Users\User\AppData\Local\Programs\<company_name_here>\resources\app.asar\main instead of just main by replacing everything that comes before .asar\.

The data I'm looking at may have no source maps or lack of context lines.

This causes grouping issues downstream. You can validate this in an Electron project with Discover by querying for stack.module:"C:\Users*" and using the following columns (see screenshot).
image

I see the following values:

  • module: `C:\Users\User\AppData\Local\Programs\<company_name_here>\resources\app.asar\main
  • filename: app:///main.js
  • abs_path: app:///main.js

I would expect the values to be:

  • module: main
  • filename: app:///main.js
  • abs_path: C:\\Users\\User\\AppData\\Local\\Programs\\<company_name_here>\\resources\\app.asar\\main.js

Q: Would this be a reasonable fix?

@armenzg
Copy link
Member Author

armenzg commented Jun 13, 2023

From the original issue:

module in stacktraces include the user account's installation path, thus, causing groups to be created per user.

This is snippet is from project 5599205 and group 4019798020 (redacted name of user):

{
"function": "IncomingMessage.<anonymous>",
"module": "C:\\Users\\NameOfUser\\AppData\\Local\\Programs\\AppName\\resources\\app.asar\\main",
"filename": "app:///main.js",
"abs_path": "app:///main.js",
"lineno": 449,
"colno": 544,

If you use Discover and filter for title:"HttpError: 403 Forbidden" and group per unique issues you will see that 35k groups were created for 11M events in the last 90 days.

Image

@lforst
Copy link
Member

lforst commented Jun 13, 2023

Hi, the project you shared uses quite an old version of the electron SDK (2.5.4). The current version of the SDK already normalizes/strips away user paths for grouping and PII reasons.

@timfish
Copy link
Collaborator

timfish commented Jun 13, 2023

You can validate this in an Electron project with Discover by querying for stack.module:"C:\Users*"

I've looked through my production apps and can't find anything so I would definitely try updating to the SDK to the latest first.

The current version of the SDK already normalizes/strips away user paths for grouping and PII reasons

It should but this might be something we're missing in some cases with module. We don't attempt to normalise that since I've never seen it containing absolute paths!

@armenzg, if you're still getting issues with the latest SDK, can you supply the full, raw, minified stack string (with username redacted) and I should be able to get a better idea of what's going on. You can get that by ticking these two boxes:

image

@armenzg
Copy link
Member Author

armenzg commented Jun 14, 2023

@timfish I'm reporting on behalf of the customer, thus, I can't upgrade their SDK.

I've got the analytics showing how many groups got created on June 1st by SDK version and project ID.
You can see the spreadsheet here.

Snapshot:
image

Query:

SELECT
  sdk_name, sdk_version, project_id,
  COUNT(DISTINCT group_id) AS cnt
FROM `super-secure-big-data.streams_partitioned.errors_gcs`,
UNNEST(exception_frames_module) AS m,
UNNEST(contexts_key) AS ckey WITH OFFSET c1,
UNNEST(contexts_value) AS cvalue WITH OFFSET c2
WHERE dt = '2023-06-01'AND 
      c1 = c2 AND 
      ckey = "runtime.name" AND cvalue = "Electron" AND STARTS_WITH(m, "C:\\Users")
GROUP BY 1,2,3
ORDER BY 4 DESC

@timfish
Copy link
Collaborator

timfish commented Jun 15, 2023

@armenzg are you able to supply an example of the full, raw, minified stack string with username redacted?

@armenzg armenzg changed the title For the module field, exclude anything before .asar/ Electron - For the module field, exclude anything before .asar/ Jun 16, 2023
@armenzg
Copy link
Member Author

armenzg commented Jun 16, 2023

@timfish Does this help? Thanks for looking into this!
I wonder if the value of the culprit is used as part of the module when the file is app:///main.js

HttpError: 403 Forbidden
"method: GET url: https://s3-us-west-2.amazonaws.com/<redacted>/desktop-packages/latest.yml?noCache=1h1mini3k\n\n          Data:\n          <?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>AccessDe...
  File "app:///main.js", line 427, col 4694, in s
    {snip} 41659),n=c.default("electron-builder");function s(_,T=null){return new r(_.statusCode||-1,`${_.statusCode} ${_.statusMessage}`+(T==null?"":`
  File "app:///main.js", line 432, col 544, in IncomingMessage.<anonymous>
    {snip} .find(Ne=>Ne.includes("json"))!=null:J.includes("json"));I(s(T,`method: ${O.method||"GET"} url: ${O.protocol||"https:"}//${O.hostname}${O.po {snip}
  File "events.js", line 315, col 20, in IncomingMessage.emit
  File "domain.js", line 467, col 12, in IncomingMessage.EventEmitter.emit
  File "internal/streams/readable.js", line 1327, col 12, in endReadableNT
  File "internal/process/task_queues.js", line 80, col 21, in processTicksAndRejections
{
"culprit": "s(C:\\Users\\<redacted>\\AppData\\Local\\Programs\\<redacted>\\resources\\app.asar\\main)",
"exception": {
    "values": [
    {
    "stacktrace": {
        "frames": [
            {
            "function": "IncomingMessage.<anonymous>",
            "module": "C:\\Users\\<redacted>\\AppData\\Local\\Programs\\<redacted>\\resources\\app.asar\\main",
            "filename": "app:///main.js",
            "abs_path": "app:///main.js",
            "lineno": 432,
            "colno": 544,
            "pre_context": [
                "{snip} efineProperty(w,\"__esModule\",{value:!0}),w.CancellationError=w.CancellationToken=void 0;const e=t(82            361);class c extends e.EventEmitter{cons {snip}",
                "`+JSON.stringify(T,null,\"  \"))+`",
                "{snip} tusCode=T,this.description=N,this.name=\"HttpError\",this.code=`HTTP_ERROR_${T}`}isServerError(){return this.statusCode>=500&&this.statusCode< {snip}",
                "",
                "Please double check that your authentication token is correct. Due to security reasons, actual status maybe not reported, but 404."
            ],
            "context_line": "{snip} .find(Ne=>Ne.includes(\"json\"))!=null:J.includes(\"json\"));I(s(T,`method: ${O.method||\"GET\"} url: ${O.protocol||\"https:\"}//${O.hostname}${O.po {snip}",
            "post_context": [
                "",
                "          Data:",
                "          ${_e?JSON.stringify(JSON.parse(G)):G}",
                "{snip} owed size is 500 MB\"));return}F=Buffer.alloc(G),K=0}}V.on(\"data\",G=>{if(K!==-1)G.copy(F,K),K+=G.length;else if(F==null)F=G;else{if(F.length> {snip}",
                "`).join(`"
            ],
            "in_app": true
},

@timfish
Copy link
Collaborator

timfish commented Jun 21, 2023

I've run some tests with the getModule function that gets the module name from a filename and it correctly parses the above paths so I'm not sure what's causing this yet.

@timfish
Copy link
Collaborator

timfish commented Jun 29, 2023

I still have no idea what's causing this but you can disable the module parsing for stack frames in the main process by overriding the stackParser option with the node parser but without module parsing:

import { createStackParser, nodeStackLineParser } from '@sentry/utils';
import { init } from '@sentry/electron/main';

init({
  dsn: '__DSN__',
  stackParser: createStackParser(nodeStackLineParser())
});

If I'm supplied a minimal reproduction of this issue that I can debug locally I might have a better chance of tracking down how this is happening.

@armenzg
Copy link
Member Author

armenzg commented Jul 7, 2023

Hi @timfish is there a demo app for Mac I can tweak?
Do you have a way to try with the same SDK version they're using?

Also, taking note that I believe this is lacking source maps, this is why the context line is full of snip values:
image

@armenzg
Copy link
Member Author

armenzg commented Jul 10, 2023

This issue may improve the cases where context-line is too long. If that fix makes the context-line not to be excluded we will not use the module for grouping purposes, thus, not hit this issue as much.

Nevertheless, we still have the issue where the module includes the user path.

AbhiPrasad pushed a commit to getsentry/sentry-javascript that referenced this issue Jul 11, 2023
I'm looking to fix [this Electron
issue](getsentry/sentry-electron#686) and need
access to the `getModule` function but it's not currently exported.

Since we're now exporting this, I've renamed it to
`getModuleFromFilename` since that better describes what it does.
@AbhiPrasad AbhiPrasad reopened this Jul 11, 2023
@AbhiPrasad
Copy link
Member

re-opened this as electron sdk change still needs to be made after we release JS SDK.

@timfish
Copy link
Collaborator

timfish commented Jul 13, 2023

This should be fixed in v4.8.0 which will be released shortly!

@armenzg
Copy link
Member Author

armenzg commented Jul 19, 2023

@timfish thank you for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment