search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

dartdevc: toString() crashes on non-Dart object #35952

Open rakudrama opened 5 years ago

rakudrama commented 5 years ago

A customer reports the following stack trace when trying to interpolate a GwtException from a GWT app in the page (paths anonymized).

Clearly it should not crash. It seems like the dart.str helper should be hardened. Opened #35951 to answer the separate question of what it should do.

TypeError: obj[$toString] is not a function
    at Object.dart_library.library.dart.str (https://.../resources/dart_sdk.js)
    at Object.dart_library.library.expression9.temporaryFunction82436245992617 (http://dev_compiler_debugger/last.js:21:17)
    at eval (eval at _onException (https://.../resources/packages/.../exceptions.js), <anonymous>:1:71)
    at eval (eval at _onException (https://.../resources/packages/.../exceptions.js), <anonymous>:1:324)
    at dart_library.library.exception_handler.ConsoleExceptionHandler.new._onException (https://.../resources/packages/.../exceptions.js)
    at _onException.next (<anonymous>)
    at runBody (.../resources/dart_sdk.js)

There is a thread with the title 'Check if object is "NativeJavaScriptObject"' with more details.

rakudrama commented 5 years ago

Note that Object.create(null).toString() fails with a TypeError.

jmesserly commented 5 years ago

Yeah, it's an object without Object.prototype in the proto chain. This is an issue for all members on dart:core Object. The question is to what extent we want to slow them down.

Clearly it should not crash. It seems like the dart.str helper should be hardened. Note that Object.create(null).toString() fails with a TypeError.

Yeah, JavaScript toString() behavior is identical: it throws a TypeError. It's not clear to me we need to change this behavior (and I wouldn't describe it as a "crash"--it is an unsupported operation on those objects). We may want to have friendlier behavior than JS does, especially for toString() which is used for error reporting. (Conceptually it's nice if toString() never throws... but there's no way to guarantee that in Dart or JS.) Edit: note that DDC is not currently calling obj.toString(), but rather making assumptions about the prototype chain. My current thoughts on #35951 is that it should do .toString() like JS does, which may fix this case, if GWT exposes a toString method on their objects.

I'll think more on this in the other bug. I would be hesitant to implement the other Object members, especially hashCode and == as that may incur too much performance cost.

Regarding the stack trace, it is interesting that "dev_compiler_debugger", "eval" and "temporaryFunction82436245992617" are on the stack. This looks like it is using the Dart debugger extension code somehow. @alan-knight does that stack trace look familiar at all? I found the corresponding internal code for "ConsoleExceptionHandler" (can send you link, it's easy to find though) but I don't understand where the "eval" and "dev_compiler_debugger" stuff is coming from.

jmesserly commented 5 years ago

(marked blocked, as we'll need to address https://github.com/dart-lang/sdk/issues/35951 first.)