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

RuntimeTypeInspector.js: Add RTI build target + Examples integration #5817

Draft
wants to merge 73 commits into
base: main
Choose a base branch
from

Conversation

kungfooman
Copy link
Collaborator

@kungfooman kungfooman commented Nov 10, 2023

Fixes #2529

I'm excited to introduce RuntimeTypeInspector.js, which I believe will benefit PlayCanvas as a whole and our debugging workflows in specific. RTI is designed to type-check the arguments of every function and method. It detects type errors while the engine is running in a way that goes beyond the capabilities of TypeScript, as demonstrated in this video (there is another one on the website debugging the Microsoft Holometric Video PC project).

During the development of RTI it already helped me to open issues or make PR's for various issues (e.g. NaN bugs).

I believe that RTI will help improve productivity and efficiency for both engine developers and users, by catching errors early on and reporting them in detail.

Runtime Type Inspector is implemented using Babel AST traversal with custom JSDoc parsing (powered by TypeScript itself for highest compatibility), customized to PlayCanvas requirements. That means for example supporting polymorphing this types.

TLDR: It adds type assertions for runtime type checking, for example:

image

Being able to set breakpoints exactly where the errors are happening for the first time helps to track down problems that are otherwise difficult to pinpoint (and saves a lot of time 😅).

Try for yourself: http://pc.runtimetypeinspector.org/

Just make sure to pick the right JS context - the exampleIframe:

image

I'm working on this for quite some time by now, a mix of fun and frustration - rewriting code on the AST level first seemed tricky, but given enough time, it was a rather straightforward way to automate type validations. I'm excited to read feedback and suggestions. 🙏

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@Maksims
Copy link
Contributor

Maksims commented Nov 11, 2023

This is pretty cool!
A couple of questions:

  1. Is there a way to suppress reporting more than N errors if they are originating from the same place? Spam of errors on every application update - is not much of use.
  2. Is there a way to improve the stack within console log reporting so it points to the right place of the error? This will allow easier debugging with Dev Tools, but also provide a deep linking from Launcher to IDE.

@kungfooman
Copy link
Collaborator Author

Hey @Maksims, thank you for the feedback!

  1. Is there a way to suppress reporting more than N errors if they are originating from the same place? Spam of errors on every application update - is not much of use.

Yep, for this there is this button:

image

Currently it supports a "warn once" or "spam all the time" mode, but do you mean even more fine-grained control with N + source?

(I just uploaded to latest HEAD + fixed a issue with the spam-button-state, now it saves/restores properly from localStorage)

2. Is there a way to improve the stack within console log reporting so it points to the right place of the error? This will allow easier debugging with Dev Tools, but also provide a deep linking from Launcher to IDE.

It's a console.error and if you click on it, the stack shows up:

image

Then you can click on the file location under assertType and you are at the "error source" and can just put a breakpoint:

image

So currently it takes two clicks to get to the place of the type error, but I'm not sure what you mean with deep linking from Launcher to IDE, could you elaborate a bit on that?

Thank you again!

@Maksims
Copy link
Contributor

Maksims commented Nov 11, 2023

Yep, for this there is this button:

It did not work, unfortunately. I tried it, made no difference. Still, had errors spammed:
image

It's a console.error and if you click on it, the stack shows up:

Ah, I did open dev tools after the errors were reported, it does not contain stack in such place, and all errors "originated" from the same place.
Is there a way to preserve their original stack? E.g. if the exceptions happen in the user script, but it reports first playcanvas.rti.js:504, then it is less clear where the error happened. This is actually an issue already with the current engine, where some errors like providing wrong arguments, are reported from within the engine (that's where they happen), but it is not straight away apparent to the user that it was due to passing wrong arguments. Especially when these arguments are not directly applied, but this is slightly unrelated.
It just highlights an issue: if an error is reported not from the user script, then they first think it is not their doing. So if an error happened somewhere within the user script or engine, the stack should specify errors from there, not from one single place.

Editor can try their best to filter through stack lines, and figure out if it can provide a deep link to a specific script in IDE, as it is really useful.

@kungfooman
Copy link
Collaborator Author

kungfooman commented Nov 12, 2023

It did not work, unfortunately. I tried it, made no difference. Still, had errors spammed:

Hm, maybe the intention of the button isn't clear, when "Spam type reports" is checked it means that every type error is calling console.error (and if it is unchecked, the type errors are only logged once). If that doesn't work, it is probably because I updated the file and then it needs a Ctrl+R with DevTools open to clear the cache 🤔

Thank you for pointing out the stack trace issue when DevTools isn't open from the beginning - I wasn't aware of that, so I will save the stack traces aswell for better inspection (would be nice to see in UI too).

Editor can try their best to filter through stack lines, and figure out if it can provide a deep link to a specific script in IDE, as it is really useful.

Right, I made a little experiment what kind of information we can get out of the stack trace for Editor integration:

const info = () => {
  try {
    throw new Error(); // Dummy exception to retrieve stack trace
  } catch(e) {
    const {stack} = e;
    if (stack.toLowerCase().includes('script')) {
      const lines = stack.split('\n');
      const i = lines.findIndex(_ => _.includes('ScriptComponent'));
      let scriptLine = lines[i - 1];
      scriptLine = scriptLine.replace(/at/, '').trim().split(' ')[0];
      console.warn('Type error in script: ', scriptLine);
    }
  }
}
info(), false; // false so it doesn't breakpoint here

This is "breakpoint code" we can enter here:

image

(clicking on the function source)

image

Then you have integrated the "breakpoint code" into the code flow and now you can play around with stack traces:

image

Thinking about the editor, we could just add "quick links" to the actual scripts now. Thank you for the suggestion!

Edit: I think I finally understood it: Instead of "spam" and "once", it should have an option "never", so it doesn't spam anything in the console (I will work on that).

Edit 2: Added the type error reporting mode never now:

image

(this keeps DevTools completely "clean", only reporting in the UI)

@kungfooman kungfooman marked this pull request as draft November 28, 2023 15:12
@kungfooman
Copy link
Collaborator Author

I'm finally coming to a resolution here, it just feels so spammy for many examples still, even tho they run fine (simply based on wrong types in the engine). My main goal is "0 errors" - so developers can start editing examples and only see type errors they introduced themselves during their coding session.

If anyone has time, I would love to have these issues fixed:

Any volunteers? 😅

@LeXXik
Copy link
Contributor

LeXXik commented Jun 21, 2024

A good cause 👍 I might tackle some of those next week.

# Conflicts:
#	package-lock.json
# Conflicts:
#	package-lock.json
#	package.json
# Conflicts:
#	examples/rollup.config.mjs
#	examples/utils/plugins/rollup-build-examples.mjs
# Conflicts:
#	examples/package-lock.json
#	examples/package.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate function parameter validation for public Engine API
4 participants