fzyzcjy / dart_interactive

REPL (interactive shell) for Dart, supporting 3rd party packages, hot reload, and full grammar
https://github.com/fzyzcjy/dart_interactive
MIT License
147 stars 12 forks source link

"Hot reload failed" all the time after attempting to import http #88

Closed Maritsu closed 9 months ago

Maritsu commented 11 months ago

Describe the bug After attempting to import package:http/http.dart, for all inputs I get Error: Hot reload failed, maybe because code has syntax error?, even if the code is syntactically correct.

To Reproduce Steps to reproduce the behavior:

  1. Install the shell using dart pub global activate interactive
  2. Open the shell using interactive
  3. !dart pub add http
  4. import 'package:http/http.dart Any line from now on will error out with Error: Hot reload failed, maybe because code has syntax error?

Expected behavior The code executes correctly, and the library is imported.

Screenshots image

Desktop (please complete the following information):

Additional context The issue only occurs after attempting to import ANY package.

welcome[bot] commented 11 months ago

Hi! Thanks for opening your first issue here! :smile:

fzyzcjy commented 11 months ago

Hmm what about checking the internals, i.e. the auto generated dart code file. Look at it manually and see what is going wrong etc.

JordyScript commented 10 months ago

This Error occurs for me too when importing any third party package (i.e. any package whose URI starts with: package:).

Local imports work fine (e.g. import 'foo.dart';, except when using a part directive #86 ) and standard library imports (e.g. import 'dart:io';) work fine too.

With regards to the generated dart code, it does have a syntactically valid import statement to the third party package, and the package is available (i.e. it is properly installed and can be imported in other scripts).

fzyzcjy commented 10 months ago

when importing any third party package

That's weird. I have mentioned this in my README:

>>> !dart pub add path // normal shell command
>>> import 'package:path/path.dart'; // normal import
>>> join('directory', 'file.txt') // use it (`join` is a function in 3rd party package `path`)
directory/file.txt

which worked well (at least when I tried it when developing it). could you please try these lines of code and see whether they works?

JordyScript commented 10 months ago

I've tried out your suggestion and here are my results. First of all a note on methodology, after every failed import / hot reload error I made sure to exit the interpreter, because even imports that would normally work would fail to hot reload after the first failure in the same interpreter session. (perhaps this snowballing effect of failures should be documented in the readme as well?)

so `path` works ``` >>> !dart pub add path "path" is already in "dependencies". Will try to update the constraint. Resolving dependencies... _fe_analyzer_shared 63.0.0 (64.0.0 available) analyzer 6.1.0 (6.2.0 available) Got dependencies! >>> import 'package:path/path.dart'; ```
But `http` doesn't work ``` >>> !dart pub add http Resolving dependencies... _fe_analyzer_shared 63.0.0 (64.0.0 available) analyzer 6.1.0 (6.2.0 available) + http 1.1.0 Changed 1 dependency! >>> import 'package:http/http.dart'; [WARNING 2023-08-15 14:28:41.396920] Error: Hot reload failed, maybe because code has syntax error? >>> ```
Other packages like `dartx` (and `json_annotation`) don't work either: ``` >>> !dart pub add dartx Resolving dependencies... _fe_analyzer_shared 63.0.0 (64.0.0 available) analyzer 6.1.0 (6.2.0 available) + characters 1.3.0 + clock 1.1.1 + dartx 1.2.0 + time 2.1.3 Changed 4 dependencies! >>> import 'package:dartx/dartx.dart'; [WARNING 2023-08-15 14:32:15.894287] Error: Hot reload failed, maybe because code has syntax error? >>> ```

What I have noticed is that all the packages that don't work have a dependency on _fe_analyzer_shared and analyzer. I assume that analyzer relies on code introspection working the way it would in a normal execution environment and that since the interpreter violates (some of) those assumptions, therefore it fails. I'm not an expert on dart vm/native, nor your interactive interpreter, nor analyzer, but this is what would make sense from the little I do know.

Since my code example in #86 also uses json_annotation which depends on analyzer, that means that by itself would cause the failure. Using a part directive without the use of any library that depends on analyzer will in fact work normally, which I have confirmed with a more minimal code example. Therefore #86 will have to be closed as part directives were not the actual culprit.

fzyzcjy commented 10 months ago

I assume that analyzer relies on code introspection working the way it would in a normal execution environment and that since the interpreter violates (some of) those assumptions, therefore it fails. Therefore https://github.com/fzyzcjy/dart_interactive/issues/86 will have to be closed as part directives were not the actual culprit.

Good job in finding the causes and it looks interesting!

IIRC, my package simply writes code as plain text into .dart file, and call dart services to "hot reload" it. Thus, I think it is possible to configure (?) it to make the environment happy about analyzer.

Maybe we can try to firstly print out more friendly errors (e.g. what causes the error), or maybe look at dart vm service logs etc. Then maybe we can dig out something interesting from the error.

JordyScript commented 10 months ago

Before I respond to your comment, another naive possibility for a more specific source of the bug would be that maybe analyzer is unable to operate from across the boundaries of an isolate? Maybe analyzer documents this sort of thing, or maybe it shouldn't matter whether you are using an isolate with it and the actual specific cause is different.

Now onto your comment: Indeed, more informative error messages would be a huge improvement, for sure.

What would also be helpful for usability of interactive is for the interpreter to roll back changes that result in an error automatically, such that errors don't snowball as they do now with each additional command reporting the same error, because fixing the error would require you to manually edit the generated file (which hopefully would not be overwritten with the original error from the in memory representation of the generated file when it is next generated) or just quit and restart the interpreter.

Together these improvements would significantly improve user experience because the user would learn from the error messages what not to do and would no longer have to start from scratch with a fresh interpreter process after every error.

IIRC, my package simply writes code as plain text into .dart file, and call dart services to "hot reload" it. Thus, I think it is possible to configure (?) it to make the environment happy about analyzer.

This is difficult for me to reason about. Admittedly because I have yet to properly read the code of interactive and because I currently lack experience with language design and interpreter design.

The organized part of me would want to reason about the fundamental architecture of interactive to understand whether we are running into a bug or a wall here, i.e. can this be fixed by altering the code to account for X or this problem a fundamental limitation of the current architecture. If the latter, then what would an appropriate architecture that can support all language features in an interactive interpreter look like? or are the language features incompatible with an interactive interpreter, such that there will always be X% of corner cases that don't work interactively (i.e. some packages will never work).

The pragmatic part of me just wants to hack away at what's currently there and figure out by trial and error what is possible, the quick and dirty way.

The realistic part of me knows that I'll struggle to find time to act on either one of them, and that I can't necessarily justify the time commitment because an interactive interpreter for dart is a nice to have, but not a must have for me personally, so ranks lower than my daily responsibilities.

However, I may come back to this for the sheer enjoyment of programming, and to hone my programming skills, and just to help out because I appreciate that you may be in a very similar position of struggling to find the time and to justify working on this project and it's nicer for everyone if we help each other.

I guess at some point someone will have to do the architectural work to reason about what architecture is capable of supporting the full language spec, if any, and how best to go about building that, and subsequently document that. It may very well be that your current approach is the right one both in terms of its capacity to support the full language spec and also in terms of its implementation simplicity (making good use of upstream code), but I don't take that for granted. Or maybe I'm just overthinking it :)

fzyzcjy commented 10 months ago

is for the interpreter to roll back changes that result in an error automatically

That looks good!

The organized part of me would want to reason about the fundamental architecture of interactive to understand whether we are running into a bug or a wall here

The architecture, shortly speaking, is like:

The realistic part of me knows that I'll struggle to find time to act on either one of them, and that I can't necessarily justify the time commitment because an interactive interpreter for dart is a nice to have, but not a must have for me personally, so ranks lower than my daily responsibilities. However, I may come back to this for the sheer enjoyment of programming, and to hone my programming skills, and just to help out because I appreciate that you may be in a very similar position of struggling to find the time and to justify working on this project and it's nicer for everyone if we help each other.

I can totally understand that, yes I am also having quite limited time on this project (especially since I need to maintain all my other open source projects). Totally agree :)

I guess at some point someone will have to do the architectural work to reason about what architecture is capable of supporting the full language spec, if any, and how best to go about building that, and subsequently document that. It may very well be that your current approach is the right one both in terms of its capacity to support the full language spec and also in terms of its implementation simplicity (making good use of upstream code), but I don't take that for granted. Or maybe I'm just overthinking it :)

As mentioned above, it just utilizes hot-reload + execute a method using VM. So I do expect it to work as long as hot reload works, but yes there can be problems that we have not realized.

davidmartos96 commented 9 months ago

Attempt to fix this here #89

JordyScript commented 9 months ago

Thanks for fixing this :) Now all we need is a version bump and new release on pub.dev

fzyzcjy commented 9 months ago

a version bump and new release on pub.dev

Done!

Chematronix commented 3 months ago

Hi, just discovered and installed Interactive v1.3.0 in Winbugs, and this is still happening:

PS D:\Dev\Flutter\db_admin> C:\Users\Chema\AppData\Local\Pub\Cache\bin\interactive.bat
Run: C:\bin\flutter\bin\cache\dart-sdk\bin\dart.exe [--enable-vm-service=58927, file:///C:/Users/Chema/AppData/Local/Pub/Cache/global_packages/interactive/bin/interactive.dart-3.3.0.snapshot, --vm-service-was-enabled]
The Dart VM service is listening on http://127.0.0.1:58927/1QOl6qQVj5k=/
The Dart DevTools debugger and profiler is available at: http://127.0.0.1:58927/1QOl6qQVj5k=/devtools?uri=ws://127.0.0.1:58927/1QOl6qQVj5k=/ws
Workspace: C:\Users\Chema\AppData\Local\Temp\dart_interactive_workspace_2024-03-11T180524931417
>>> import 'dart:io';
[InstanceRef id: objects/null, kind: Null, identityHashCode: -1, classRef: [ClassRef id: classes/172, name: Null, library: [LibraryRef id: libraries/@0150898, name: dart.core, uri: dart:core]]]
import 'dart:math';
[InstanceRef id: objects/null, kind: Null, identityHashCode: -1, classRef: [ClassRef id: classes/172, name: Null, library: [LibraryRef id: libraries/@0150898, name: dart.core, uri: dart:core]]]
>>> import 'package:intl/intl.dart';
[WARNING 2024-03-11 18:05:36.632863] Error: Hot reload failed, maybe because code has syntax error?
>>> import 'package:test/test.dart';
[WARNING 2024-03-11 18:05:40.341360] Error: Hot reload failed, maybe because code has syntax error?
>>> print(1)
[WARNING 2024-03-11 18:14:21.550544] Error: Hot reload failed, maybe because code has syntax error?

I can see the workaround in executor.dart:

    if (Platform.isWindows) {
      forceReload = true;
    }

and of course:

>>> import 'dart:io';
[InstanceRef id: objects/null, kind: Null, identityHashCode: -1, classRef: [ClassRef id: classes/172, name: Null, library: [LibraryRef id: libraries/@0150898, name: dart.core, uri: dart:core]]]
>>> Platform.isWindows
true

Is something else required to get it to import package:.*?

fzyzcjy commented 3 months ago

Hi, firstly, have you pub add those packages?

Chematronix commented 3 months ago

Hi, thanks for answering.

Hi, firstly, have you pub add those packages?

Yeah. It's a Flutter project tho, does that matter?

PS D:\Dev\Flutter\db_admin> flutter pub add intl    
"intl" is already in "dependencies". Will try to update the constraint.
[...]

PS D:\Dev\Flutter\db_admin> C:\Users\Chema\AppData\Local\Pub\Cache\bin\interactive.bat
Run: C:\bin\flutter\bin\cache\dart-sdk\bin\dart.exe [--enable-vm-service=50963, file:///C:/Users/Chema/AppData/Local/Pub/Cache/global_packages/interactive/bin/interactive.dart-3.3.0.snapshot, --vm-service-was-enabled]
The Dart VM service is listening on http://127.0.0.1:50963/9CDXtu75vlk=/
The Dart DevTools debugger and profiler is available at: http://127.0.0.1:50963/9CDXtu75vlk=/devtools?uri=ws://127.0.0.1:50963/9CDXtu75vlk=/ws
Workspace: C:\Users\Chema\AppData\Local\Temp\dart_interactive_workspace_2024-03-12T174629976569
>>> import 'package:intl/intl.dart';
[WARNING 2024-03-12 17:46:34.533063] Error: Hot reload failed, maybe because code has syntax error?
fzyzcjy commented 3 months ago

https://github.com/fzyzcjy/dart_interactive#execute-within-environment-of-existing-package

Hmm, have you tried this --directory

In addition, to isolate the issue, maybe try to run https://github.com/fzyzcjy/dart_interactive#demo-1-demonstrate-features 's demos and see whether they works as well

Chematronix commented 3 months ago

Thanks, that does allow me to import pub packages.

But there are a number of issues that make it basically unusable in Windows:

  1. Using backspace, trying to print() or importing a .dart with print() all provoke Error: Hot reload failed and then all commands will produce the same error, so gotta restart the repl.
  2. Can't move on the command line with arrows nor Ctrl+B/F.
  3. No command history with arrows or ^P/^N either.

but all this works fine in Linux, including WSL, so you might want to point Windows users to go straight to WSL or their closest *nix.

Other things that would be handy to have noted in the docs: import is relative to Workspace/lib. import will error if it doesn't end with a semicolon.

--directory needs a full path, else I get:

$ interactive --directory drtst
...
Workspace: drtst
Unhandled exception:
IsolateSpawnException: Unable to spawn isolate: .pub-cache/hosted/pub.dev/interactive-1.3.0/bin/drtst/lib/auto_generated.dart: Error: Error when reading '.pub-cache/hosted/pub.dev/interactive-1.3.0/bin/drtst/lib/auto_generated.dart': No such file or directory

Seems most errors will break the repl, so gotta restart it (a kb shortcut would be nice):

>>> print()
[WARNING 2024-03-13 01:50:18.419137] Error: Hot reload failed, maybe because code has syntax error?
>>> 1
[WARNING 2024-03-13 01:50:20.464239] Error: Hot reload failed, maybe because code has syntax error?

Still, this will be very handy for learning Dart (and the one advantage it seems to have over ClojureDart xD). Thanks a bunch!

fzyzcjy commented 3 months ago

You are welcome! Feel free to PR to modify the README (no worries about wording or details, I can refine them)