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

nvim-dap will JSON encode very large integers in scientific notation #1004

Closed
christopherfujino opened this issue Jul 31, 2023 · 6 comments
Closed
Labels

Comments

@christopherfujino
Copy link

Debug adapter definition and debug configuration

  local dap = require('dap')

  dap.adapters.dart = {
    type = "executable",
    command = "dart",
    args = {"debug_adapter"}
  }
  dap.configurations.dart = {
    {
      type = "dart",
      request = "launch",
      name = "Launch Dart Program",
      program = "${workspaceFolder}/main.dart",
      cwd = "${workspaceFolder}",
    }
  }

Debug adapter version

d17d1bb

Steps to Reproduce

Have the server send a request with an integer large enough that lua's internal tostring() implementation would return a string in scientific notation, such as:

print(vim.json.encode({big_int = 3053700806959403}))
-- {"big_int":3.0537008069594e+15}

My exact case was my dart debug-adapter server sent the following body string:

{"seq":11,"type":"event","body":{"reason":"started","threadId":3053700806959403},"event":"thread"}

While lua doubles have enough precision to store this number accurately, tostring() will use scientific notation, for convenience. Thus, when the nvim-dap client re-encodes this ID number to send a future request to the server, such as this:

[ DEBUG ] 2023-07-31T11:13:58Z-0700 ] ...o/.local/share/nvim/plugged/nvim-dap/lua/dap/session.lua:1609 ]	"request"	{
  arguments = {
    threadId = 3.0537008069594e+15
  },
  command = "stackTrace",
  seq = 6,
  type = "request"
}

The server throws an exception:

[ DEBUG ] 2023-07-31T11:13:58Z-0700 ] ...o/.local/share/nvim/plugged/nvim-dap/lua/dap/session.lua:938 ]	1	"{\"seq\":16,\"type\":\"response\",\"body\":{\"error\":{\"format\":\"{message}\\n{stack}\",\"id\":1,\"variables\":{\"message\":\"type 'double' is not a subtype of type 'int' in type cast\",\"stack\":\"#0      new StackTraceArguments.fromMap (package:dap/src/protocol_generated.dart:5650:36)\\n#1      StackTraceArguments.fromJson (package:dap/src/protocol_generated.dart:5635:27)\\n#2      BaseDebugAdapter.handle (package:dds/src/dap/base_debug_adapter.dart:118:21)\\n#3      BaseDebugAdapter.handleIncomingRequest (package:dds/src/dap/base_debug_adapter.dart:419:7)\\n#4      BaseDebugAdapter._handleIncomingMessage (package:dds/src/dap/base_debug_adapter.dart:295:7)\\n#5      ByteStreamServerChannel._readMessage (package:dds/src/dap/protocol_stream.dart:82:18)\\n#6      ByteStreamServerChannel.listen.<anonymous closure> (package:dds/src/dap/protocol_stream.dart:53:24)\\n#7      _RootZone.runUnaryGuarded (dart:async/zone.dart:1594:10)\\n#8      _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:339:11)\\n#9      _DelayedData.perform (dart:async/stream_impl.dart:515:14)\\n#10     _PendingEvents.handleNext (dart:async/stream_impl.dart:620:11)\\n#11     _PendingEvents.schedule.<anonymous closure> (dart:async/stream_impl.dart:591:7)\\n#12     _microtaskLoop (dart:async/schedule_microtask.dart:40:21)\\n#13     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)\\n#14     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:118:13)\\n#15     _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:185:5)\\n\"}}},\"command\":\"stackTrace\",\"message\":\"type 'double' is not a subtype of type 'int' in type cast\",\"request_seq\":6,\"success\":false}"

TL;DR the server crashes because it receives a double where an int was expected.

The only solution I can think of to fix this is to write a custom json encoder that will test if numbers are integers, and if so ensure they are stringified as such to ensure no loss of precision (even if the server were to be more lax, if you notice in this example, two decimal points of precision are lost by using scientific notation).

To unblock myself, I will write a custom lua encoder. @mfussenegger I'm curious whether you would be interested in upstreaming it, or if this is such a nice bug that I should just maintain my own fork.

Expected Result

Breakpoints can be hit without the server crashing.

Actual Result

See above, the server receives scientific notation which it interprets as a double, and a cast to int fails.

@christopherfujino
Copy link
Author

related upstream dart DAP server bug, although I think we need both to be fixed. That bug is to ensure that our threadIds never exceed 2^53, which lua doubles could not accurately track, and this bug is to ensure that below that threshold we never JSON encode ints to scientific notation.

@christopherfujino
Copy link
Author

@mfussenegger
Copy link
Owner

The only solution I can think of to fix this is to write a custom json encoder that will test if numbers are integers, and if so ensure they are stringified as such to ensure no loss of precision

Converting them to a string could break other debug adapters that expect a number. For example in Haskell:

print $ (decode "{\"threadId\": 3053700806959403}" :: Maybe Foo)
-- Just (Foo {threadId = 3053700806959403})

print $ (decode "{\"threadId\": 3.0537008069594e+15}" :: Maybe Foo)
-- Just (Foo {threadId = 3053700806959400})

print $ (decode "{\"threadId\": \"3053700806959403\"}" :: Maybe Foo)
-- Nothing

So this is not an option.

I created an issue in the debug adapter protocol repository to clarify the allowed ranges:

microsoft/debug-adapter-protocol#422

Depending on that we may need some way in Neovim to configure the number precision. There is functionality in cjson, but it's currently disabled/not exposed:

https://github.com/neovim/neovim/blob/9b5f58185e1ff0597c7e95b7205d9ec11be1848c/src/cjson/lua_cjson.c#L358-L365

@christopherfujino
Copy link
Author

christopherfujino commented Aug 1, 2023

Oh yeah, you're right. Why is this even working? lol

No, I'm not wrapping the int in quotes. I'm just creating a string for the overall json message.

@christopherfujino
Copy link
Author

That having been said, it would be great if we could configure this and didn't have to do it in lua :) https://github.com/neovim/neovim/blob/9b5f58185e1ff0597c7e95b7205d9ec11be1848c/src/cjson/lua_cjson.c#L358-L365

@mfussenegger
Copy link
Owner

Closing this here. I don't intend to add an alternative json encoder/decoder. A fix would need to go into neovim core: neovim/neovim#24532

@mfussenegger mfussenegger closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants