Recent flakes in webdev CI show failures to load dill files
due to broken sockets (most likely caused by connection timeouts).
This change adds retries and timeouts to the asset server file
system to make it tolerant to breaking connections.
- Add RetryTimeoutClient wrapper for HttpClient that performs
retries with delays and connection/response timeouts, based
on the idea in
https://github.com/dart-lang/http/blob/master/lib/retry.dart
- Make AssetFileSystem use the RetryTimeoutClient.
- Add tests for reading multiple files concurrently.
- Add tests for ExpressionCompilerWorker reading assets using
standard, multiroot, and asset file systems.
- Simplify uri that AssetFileSystem uses to query the asset
server, since we no longer need to load package uris.
This will eliminate a step in uri translation in dwds.
- Validated that changes do not break google3 manually.
Change-Id: I2ff163b8d99cd0b7551f3f33a71e6b306540e3c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204880
Commit-Queue: Anna Gringauze <annagrin@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Adding explicit source maps to the operator created a breakable location
when viewing source-mapped dart code in chrome devtools. There isn't a
breakable point in the javascript for a `!` operator so trying to enable
the breakpoint visible in the dart code would trigger some "other
breakpoint".
Recent changes in the V8 debugger have changed which "other breakpoint"
would be triggered.
https://chromium.googlesource.com/v8/v8/+/16b0767adc54de6e0154a0a8226e01efecfdbfbb
This causes our tests to fail and makes the user
interaction a little confusing. Removing the extra source map here seems
to be the most reasonable solution because it prevents a possible
breakpoint from appearing where there isn't a breakable location.
Now, the ! operator doesn't introduce any new source mapping it simply
continues the previous mapping.
Change-Id: I5903bfeaf1bcb2e2a3ce883433153575aa1fd05a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204742
Reviewed-by: Mark Zhou <markzipan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
We are seeing CI flakes in webdev when using expression_compiler_worker
in tests involving expression evaluation, for example, see linux tests
for dwds in the following PR:
https://github.com/dart-lang/webdev/pull/1343
The logs show that HttpFileSystemEntity.exists fails due to a broken
pipe, which we suspect is due to http client closed too early.
This change is an attempt to fix the problem by
- logging exceptions thrown in expression compiler worker
- awaiting for the request to be read before closing http client in
HttpFileSystemEntity.
TEST=pkg/dev_compiler/test/expression_compiler/asset_file_system_test.dart
See investigation of the CI issue in:
https://github.com/dart-lang/webdev/issues/1345
Closes: https://github.com/dart-lang/sdk/issues/46388
Change-Id: Iaf98f08e8ebb618bf8365bd497f01ae685f0d3f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203841
Commit-Queue: Anna Gringauze <annagrin@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
This is a temporary fix to give debug tools a recognizable string so
they can hide the type variables from the current scope.
The prefix is only applied when the type appears outside of the
top level type table.
This will require a change in dwds to filter variables with names that
begin with this string.
Change-Id: I2942f462280195d81a7de2b710e6b958770d152a
Issue: https://github.com/dart-lang/sdk/issues/45649
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199581
Reviewed-by: Anna Gringauze <annagrin@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Safari has a bug that makes it a syntax error for a function name
to overlap with names of parameters in functions with default
parameter values.
This CL changes DDC to generates code to circumvent this issue
when emitting top-level methods. Fortunately, the Safari bug
doesn't trigger with ES6 methods, so we don't need to do anything
for Dart instance methods.
The only other occurrence of named functions in DDC are
`function*` generators that we emit. Those take no arguments,
so we don't need any additional renaming there.
Fixes#43520
Change-Id: I2e4588701a294a8f3c5b47956826ada4ed973e6c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198200
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Marks `Function` as an extensible type so that DDC knows to use the
symbolized version of the equals operator.
The `Function` class at runtime already has its equals method attached
as the "symbolized" version. Consider `a == b`. When DDC knows `a` is
statically a raw `Function` and non-nullable (more likely with sound
null safety) the generated code should call the symbolized equals
member ex: `a[$_equals](b)`. Without this change the generated code
would be `a.equals(b)` and fail at runtime because the method does
not exist.
With this change co19_2/LibTest/core/Function/operator_eq_A01_t01
starts passing.
Change-Id: I80dd2abbbb04f1b7ab7e21dd14561a45f6e81459
Fixes: https://github.com/dart-lang/sdk/issues/45601
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194204
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
- Fixes calls that pass `when` or `message` arguments
- Updates location information on the expression statement versions to
point to the beginning of the `debugger()` call instead of the `;` at
the end of the line.
Change-Id: I56e6d3358a4a4f0f04b4087d062c513fa6729bd3
Issue: https://github.com/dart-lang/sdk/issues/45544
Change-Id: I31838b5877d139554e3aa39c9833fd83ae729a0e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195053
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
The web compilers don't support getter/field invocation encoded as a
FunctionInvocation on an InstanceGet because it doesn't work for
getter/field invocation of js-interop properties, since the InstanceGet
wouldn't result in a Dart function but just JavaScript function.
To support this in the new method invocation encoding, a special
expression, InstanceGetterInvocation, is used to encode getter/field
invocations in dart2js and ddc.
Change-Id: I21da8e8686f66ae4ce4d44245073b9e424f975b9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192181
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
In some cases, such as file system exceptions, ExpressionCompilerWorker
fails to start but does not close its receiver port, which causes the
consumer to wait indefinitely for responses.
Make ExpressionCompilerWorker communicate the failure to the consumer,
close the receiver port, and exit.
Added tests to verify graceful exit.
Issue that uncovered the bug: https://b.corp.google.com/issues/183218372
Change-Id: I54a9c3620cfc5f77c101b26b4e98b6eeb03ad7e1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192260
Commit-Queue: Anna Gringauze <annagrin@google.com>
Reviewed-by: Gary Roumanis <grouma@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Current version of dwds does not supply module format to the
expression compiler worker, resulting in initialization failures.
Add default 'amd' format so dwds continues to work with new SDK
version before a matching update that passes format to dwds
lands.
Change-Id: I10e8ddc09b48b91c9c085a6d7579ad01c6e13399
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/190820
Reviewed-by: Gary Roumanis <grouma@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Anna Gringauze <annagrin@google.com>
The CL is a step towards have a more restricted and wellstructured
handled of references and canonical names.
The CL moves Reference to canonical_name.dart and makes
CanonicalName.reference private, and replaces CanonicalName.getReference
with a 'reference' getter.
It also removes NamedNode.canonicalName, Field.getterCanonicalName and
Field.setterCanonicalName so that these can only be accessed through the
corresponding reference. This is to reduce the reliance on the
canonical names which, ideally, should only be part of serialization and
deserialization.
TEST=existing
Change-Id: I955fb7d52d4e112d8741f7c12dcf38b74ae0c91a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/190442
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
* Add InstanceGetterInvocation for getter/field invocation in web
backends
* Add localFunction getter to LocalFunctionInvocation
* Remove isNot from EqualsCall and EqualsNull - the encoding didn't
carry its weight.
* Remove uses of Name.name
* Remove BottomType code from VM
TEST=existing
Change-Id: I99d05d35b9ef193d092cc151c99ad472dbd60834
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/188725
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
The CFEs FormattedMessage always had two getters to get the text inside
one that would supposedly give an ansi formated version of the message
and one that would supposedly give a plaintext formated version of the
message. They both returned the same string, though, which would either
be with ansi escape codes or plain text depending on the environment at
compile time.
This CL fixes that by having both messages, and letting the reporting
(i.e. whenever the message is read) decide which to use. That way we
can - for instance - report errors with color if the terminal supports
it correctly when reusing a dill (and reissuing problems, but where the
terminal support changes) and if printing the problem to an html <pre>
field (like observatory does).
It also cleans up two different implementations of whether we think
the terminal supports colors or not, by deleting one of them.
This is the second try. Patchset #1 is the original.
Patchset #2(and possibly beyond) is the changes.
TEST=Existing test suites.
Change-Id: I8e483049ce81ce1bd8e5396b588a31e0ad3a8630
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187402
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This reverts commit f63f7736c5.
Reason for revert: This somehow breaks the Windows bots. `python tools/test.py -n dartk-win-release-x64 standalone_2/io/process_shell_test`.
Original change's description:
> [cfe] Actually have both ansi and plain text formatted messages
>
> The CFEs FormattedMessage always had two getters to get the text inside
> one that would supposedly give an ansi formated version of the message
> and one that would supposedly give a plaintext formated version of the
> message. They both returned the same string, though, which would either
> be with ansi escape codes or plain text depending on the environment at
> compile time.
>
> This CL fixes that by having both messages, and letting the reporting
> (i.e. whenever the message is read) decide which to use. That way we
> can - for instance - report errors with color if the terminal supports
> it correctly when reusing a dill (and reissuing problems, but where the
> terminal support changes) and if printing the problem to an html <pre>
> field (like observatory does (1)).
>
> It also cleans up two different implementations of whether we think
> the terminal supports colors or not, by deleting one of them.
>
> (1) At least sometimes. It works - I think - only for 'evaluateInFrame',
> but that's another story (and will be fixed in a follow-up CL).
>
> TEST=Existing test suites.
>
> Change-Id: Iedaedd9a5c41458d40c23ed4b706324c004ae943
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186291
> Commit-Queue: Jens Johansen <jensj@google.com>
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
Change-Id: I0b53f943a61f76705badfead30d9e1ee35baff57
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186941
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
The CFEs FormattedMessage always had two getters to get the text inside
one that would supposedly give an ansi formated version of the message
and one that would supposedly give a plaintext formated version of the
message. They both returned the same string, though, which would either
be with ansi escape codes or plain text depending on the environment at
compile time.
This CL fixes that by having both messages, and letting the reporting
(i.e. whenever the message is read) decide which to use. That way we
can - for instance - report errors with color if the terminal supports
it correctly when reusing a dill (and reissuing problems, but where the
terminal support changes) and if printing the problem to an html <pre>
field (like observatory does (1)).
It also cleans up two different implementations of whether we think
the terminal supports colors or not, by deleting one of them.
(1) At least sometimes. It works - I think - only for 'evaluateInFrame',
but that's another story (and will be fixed in a follow-up CL).
TEST=Existing test suites.
Change-Id: Iedaedd9a5c41458d40c23ed4b706324c004ae943
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186291
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Johnni Winther