Skip to content

[analysis_server] LSP test failing on adding extension with specific name #40636

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

Closed
dcharkes opened this issue Feb 14, 2020 · 5 comments
Closed
Labels
devexp-lsp Issues with analysis server's support of Language Server Protocol legacy-area-analyzer Use area-devexp instead.

Comments

@dcharkes
Copy link
Contributor

When I add an extension DynamicLibraryExtension to dart:ffi in the mock_sdk the test below starts failing. However, when the extension is named LibraryExtension, no issue at all.

Workaround: https://dart-review.googlesource.com/c/sdk/+/135463/8..9

Failure thrown in:

final errorMessage = message is ResponseMessage
? 'An error occurred while handling the response to request ${message.id}'
: message is RequestMessage
? 'An error occurred while handling ${message.method} request'
: message is NotificationMessage
? 'An error occurred while handling ${message.method} notification'
: 'Unknown message type';
sendErrorResponse(
message,
ResponseError(
ServerErrorCodes.UnhandledError,
errorMessage,
null,
));
logException(errorMessage, error, stackTrace);

cc @DanTup

Failure log:

/=========================================================================================================================================\
| analyzer-unittest-asserts-release-linux:pkg/analysis_server/test/lsp/workspace_symbols_test broke (Pass -> RuntimeError, expected Pass) |
\=========================================================================================================================================/

--- Command "vm" (took 33.000790s):
DART_CONFIGURATION=ReleaseX64 out/ReleaseX64/dart-sdk/bin/dart --enable_asserts --ignore-unrecognized-flags --packages=/b/s/w/ir/.packages /b/s/w/ir/pkg/analysis_server/test/lsp/workspace_symbols_test.dart

exit code:
255

stdout:
[...]
00:02 �[32m+3�[0m�[31m -1�[0m: WorkspaceSymbolsTest | test_partialMatch �[1m�[31m[E]�[0m�[0m
  {
      "code": -32001,
      "message": "An error occurred while handling workspace/symbol request"
  }
  pkg/analysis_server/test/lsp/server_abstract.dart 51:7        AbstractLspAnalysisServerTest.expectSuccessfulResponseTo
  ===== asynchronous gap ===========================

[...]

stderr:
Unhandled exception:
Dummy exception to set exit code.
#0      _rootHandleUncaughtError.<anonymous closure> (dart:async/zone.dart:1114:29)
#1      _microtaskLoop (dart:async/schedule_microtask.dart:43:21)
#2      _startMicrotaskLoop (dart:async/schedule_microtask.dart:52:5)
#3      _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:118:13)
#4      _Timer._runTimers (dart:isolate-patch/timer_impl.dart:405:11)
#5      _Timer._handleMessage (dart:isolate-patch/timer_impl.dart:429:5)
#6      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

--- Re-run this test:
python tools/test.py -n analyzer-unittest-asserts-release-linux pkg/analysis_server/test/lsp/workspace_symbols_test

Full log: https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8888553385798619536/+/steps/test_results/0/logs/new_test_failures__logs_/0

@DanTup
Copy link
Collaborator

DanTup commented Feb 14, 2020

However, when the extension is named LibraryExtension, no issue at all.

I was able to repro a crash where we could try to create SymbolInformation's without a Kind - however it happens in my testing for extensions with a name, since without a name the symbol is not included.

https://dart-review.googlesource.com/c/sdk/+/135902 fixes that, though I'm not sure if you're seeing something else.

@DanTup
Copy link
Collaborator

DanTup commented Feb 14, 2020

Sorry, I mis-read the original issue. This wasn't named vs unnamed, it was DynamicLibraryExtension vs LibraryExtension.

So it is the same issue (named extensions are causing issues). The reason it didn't show up without the word Dynamic is that the test was sending a query of my which matches Dynamic because we do a fuzzy match (*m*y*).

@dcharkes
Copy link
Contributor Author

Thanks for the quick fix! =)

@DanTup
Copy link
Collaborator

DanTup commented Feb 14, 2020

There was another slight issue - there's an assert to catch when we don't map a kind, but it was firing on the bots (because they run with asserts enabled) when the underlying kind was null. I've tweaked the asserts (which are there to highlight when there's a kind that hasn't been mapped) to only fire for non-null kinds that aren't mapped too.

dart-bot pushed a commit that referenced this issue Feb 14, 2020
Bug: #40636
Change-Id: I28855dac790291a16717c20cf46c33dfae23e9ff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135902
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Danny Tuppeny <[email protected]>
@devoncarew devoncarew added legacy-area-analyzer Use area-devexp instead. devexp-lsp Issues with analysis server's support of Language Server Protocol labels Feb 14, 2020
@DanTup
Copy link
Collaborator

DanTup commented Feb 14, 2020

@dcharkes the fix has been merged. Let me know if you hit any problems putting the name back as you had it (assuming you plan to).

Also, if the fix looks good please close this - I don't have the power here and forgot to include it in the commit message. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-lsp Issues with analysis server's support of Language Server Protocol legacy-area-analyzer Use area-devexp instead.
Projects
None yet
Development

No branches or pull requests

3 participants