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

Error reporting #410

Open
jonathanolson opened this issue Mar 1, 2017 · 11 comments
Open

Error reporting #410

jonathanolson opened this issue Mar 1, 2017 · 11 comments
Assignees

Comments

@jonathanolson
Copy link
Contributor

It should be potentially possible to detect simulation errors in production sims (same way aqua tests for errors in running built sims), and then send an error report back to phet.colorado.edu.

Thus we could see what types of errors are occurring in the wild (and prioritize bug fixes/maintenance releases based on that). It may also be possible to keep some assertions (if we choose) in some built sims that would trigger a report (but not fail out the sim).

Assigned to developer meeting to discuss whether this would be helpful.

Cost to add reporting would be quite minimal (could do this like Yotta messages, and only send the first failure), so I'd recommend doing this (as long as it isn't a privacy issue, which presumably if it's just a stack-trace it wouldn't be?)

Cost to add analysis to see what errors are occurring in only the latest versions of sims (and what errors are happening most often) would require more development time, and could be done to analyze reported errors retroactively as needed.

@samreid
Copy link
Member

samreid commented Mar 2, 2017

How are you envisioning catching the errors? Wrapping Sim.step() phases in try/catch? What about the startup sequence? Or is there a cross-cutting way to intercept all errors?

@jonathanolson
Copy link
Contributor Author

Or is there a cross-cutting way to intercept all errors?

Yup

Current setup for listeners:

    // Communicate sim errors to joist/tests/test-sims.html
    if ( phet.chipper.queryParameters.postMessageOnError ) {
      window.addEventListener( 'error', function( a, b, c, d, e ) {
        var message = '';
        var stack = '';
        if ( a && a.message ) {
          message = a.message;
        }
        if ( a && a.error && a.error.stack ) {
          stack = a.error.stack;
        }
        window.parent && window.parent.postMessage( JSON.stringify( {
          type: 'error',
          url: window.location.href,
          message: message,
          stack: stack
        } ), '*' );
      } );
    }

@samreid
Copy link
Member

samreid commented Mar 2, 2017

That looks great, I think we should invest in this as an error reporting feature. Would be good to:

  • name the variables in the callback
  • make sure the callback above works properly across our platforms
  • document the code above
  • why is window.parent used for postMessage?
  • Is there any other information that should be submitted with the error report?
    • user agent?
    • webgl capability
    • other data from "Report a Problem" => "Troubleshooting information (do not edit):"
    • client machine Date.now
    • client machine locality?
  • decide where the code above should live. Perhaps an early (first?) preload?
  • add sim version information to the message

Also, let's have preliminary discussion about adding more information (not constraining ourselves to just error messages). For instance, what if we also sent a message every time the user reset the sim or created a parallel circuit, etc? Or a heartbeat message (say, every 1 minute) so we can see how long the sim was used?

@pixelzoom
Copy link
Contributor

Any privacy issues? Is user consent required to report errors back to PhET?

@jonathanolson
Copy link
Contributor Author

Any privacy issues? Is user consent required to report errors back to PhET?

Presumably similar to the current Yotta messages which are sent automatically.

@pixelzoom
Copy link
Contributor

Presumably similar to the current Yotta messages which are sent automatically.

And what is the privacy decision wrt Yotta?

@jonathanolson
Copy link
Contributor Author

And what is the privacy decision wrt Yotta?

Presumably that it is equivalent to including Google Analytics on pages, which many sites do?

@jonathanolson
Copy link
Contributor Author

#410 (comment)

Yes, we definitely wouldn't want to copy/use that ugly code (and that code should be improved).

Is there any other information that should be submitted with the error report?

Ideally we would submit things that would help diagnose problems (user agent and webgl status and things from "report a problem" sound useful, as long as they aren't privacy issues).

decide where the code above should live. Perhaps an early (first?) preload?

Yes, as early as possible would be good.

@samreid
Copy link
Member

samreid commented Mar 2, 2017

Also, let's have preliminary discussion about adding more information (not constraining ourselves to just error messages). For instance, what if we also sent a message every time the user reset the sim or created a parallel circuit, etc? Or a heartbeat message (say, every 1 minute) so we can see how long the sim was used?

From the meeting: We shouldn't add any other supplemental information until we have a need for it. It seems we have a need for startup messages and error messages. Nothing else at the moment.

@samreid
Copy link
Member

samreid commented Mar 6, 2017

@jonathanolson does this still need to be marked for developer meeting?

@jonathanolson
Copy link
Contributor Author

Presumably not.

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

No branches or pull requests

3 participants