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

fix: process is killed on linter warning #460

Closed
mrgnhnt96 opened this issue Dec 28, 2022 · 5 comments · Fixed by #592
Closed

fix: process is killed on linter warning #460

mrgnhnt96 opened this issue Dec 28, 2022 · 5 comments · Fixed by #592
Assignees
Labels
feature A new feature or request

Comments

@mrgnhnt96
Copy link

Description

A clear and concise description of what the bug is.

I am getting a warning in the console when running dart_frog dev from some of my generated code.

dart_frog dev --verbose

[meta] dart_frog_cli 0.3.1
[codegen] running pre-gen...
⠙ Serving... (82ms)[codegen] running generate...
[codegen] complete.
[process] dart --enable-vm-service .dart_frog/server.dart
✓ Running on http://localhost:8080 (0.1s)
The Dart VM service is listening on http://127.0.0.1:8181/LKaUCcJtu-M=/
The Dart DevTools debugger and profiler is available at: http://127.0.0.1:8181/LKaUCcJtu-M=/devtools/#/?uri=ws%3A%2F%2F127.0.0.1%3A8181%2FLKaUCcJtu-M%3D%2Fws
packages/domain/lib/models/user.g.dart:70:20: Warning: Operand of null-aware operation '!' has type 'String' which excludes null.
          ? _value.id!
                   ^
[process] killing process...
[process] process.kill()...
[process] killing process complete.
[process] exit(1)

Steps To Reproduce

  1. Add dependencies copy_with_extension, copy_with_extension_gen (dev), build_runner (dev)
  2. Create a basic dart object
    a. Add the @CopyWith() annotation
    b. Add a field final String id
  3. Run build_runner
  4. Run dart_frog dev

Expected Behavior

A clear and concise description of what you expected to happen.

I'd expect for this warning to be omitted. I've added in the analysis_options to exclude generated files from the linter

Screenshots

# analyis_options.yaml

include: package:very_good_analysis/analysis_options.3.1.0.yaml
analyzer:
  exclude:
    - build/**
    - "**/*.g.dart"
linter:
  rules:
    file_names: false

If applicable, add screenshots to help explain your problem.

Additional Context

Add any other context about the problem here.

# pubspec.yaml in `packages/domain`

name: domain
description: A Very Good Project created by Very Good CLI.
version: 0.1.0+1
publish_to: none

environment:
  sdk: ">=2.18.0 <3.0.0"

dependencies:
  autoequal: ^0.5.1
  copy_with_extension: ^5.0.0
  equatable: ^2.0.5
  json_annotation: ^4.7.0

dev_dependencies:
  autoequal_gen: ^0.5.1
  build_runner: ^2.3.3
  copy_with_extension_gen: ^5.0.0
  json_serializable: ^6.5.4
  mocktail: ^0.3.0
  test: ^1.19.2
  very_good_analysis: ^3.1.0

scripts: ../../derry.yaml
@mrgnhnt96 mrgnhnt96 added the bug Something isn't working as expected label Dec 28, 2022
@felangel
Copy link
Contributor

felangel commented Dec 28, 2022

Hi @mrgnhnt96 👋
Thanks for opening an issue!

This doesn’t appear to be an issue with Dart Frog. Based on the warning it looks like the generated code for your user model includes an ! when it is unnecessary because the value is not nullable. This is not something that you can omit by disabling analysis afaik. The right thing to do is to remove the unnecessary !. The same thing will happen if you run a flutter application via flutter run and the code contains an unnecessary !.

@felangel felangel added question waiting for response Waiting for follow up labels Dec 28, 2022
@felangel felangel self-assigned this Dec 28, 2022
@mrgnhnt96
Copy link
Author

Yeah, the code is being generated by a different package, but because of the warning, the process gets killed, which I don't expect.

Its worth noting, if I comment out the CopyWith() annotation, run build_runner, then run dart_frog dev it spin up the server just fine, because the generated code isn't there any more. After the server is spun up, if I un-comment out the CopyWith() annotation, run build_runner, the process does not get killed even though the warning is present

@mrgnhnt96 mrgnhnt96 changed the title fix: fix: process is killed on linter warning Dec 28, 2022
@felangel
Copy link
Contributor

Yeah, the code is being generated by a different package, but because of the warning, the process gets killed, which I don't expect.

Its worth noting, if I comment out the CopyWith() annotation, run build_runner, then run dart_frog dev it spin up the server just fine, because the generated code isn't there any more. After the server is spun up, if I un-comment out the CopyWith() annotation, run build_runner, the process does not get killed even though the warning is present

Yeah Dart Frog exits if there is anything reported to stderr initially. I can try to improve that 👍

@felangel felangel added feature A new feature or request and removed bug Something isn't working as expected question waiting for response Waiting for follow up labels Dec 28, 2022
@renancaraujo
Copy link
Contributor

After looking into this problem, here are my description of the current situation:

Fact 1:

The VM (started by dart run) prints dart warnings regardless of "ignore" comments and analyzer configuration.
This is an old known issue that can be seen here: dart-lang/sdk#34137 and here: dart-lang/sdk#46264.

These wearing messages are popped from dart run directly to stderr of the running process.

Fact 2:

dart_frog dev starts the server by running dart run under hotreload. The hot reloader runs the server before popping any message communicating the establishment of hot reload successfully.

The process with hot reload forwards the error messages to the process stderr.

Fact 3:

After calling starting the server, dart_frog dev listens for the given process's stderr. Any messages on stderr before a "successful hot reload" kills the process.

That is our problem.

Proposed solution

Considering that we cannot disable these warnings and that the VM can and should pop messages to stderr for problems with higher severity than WARNING (see this file),

We should filter warning messages from stdrrr as criteria for killing the process. The affected logic is here.


To reproduce this issue locally, one should follow OP's steps but lock copy_with_extension to 5.0.0.

@renancaraujo
Copy link
Contributor

Added a quick proposal for fix here: #592

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants