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

christopherfujino commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

FYI this is my custom json encoding logic: https://github.com/mfussenegger/nvim-dap/compare/master...christopherfujino:nvim-dap:dev?expand=1

mfussenegger commented 11 months ago

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:

https://github.com/microsoft/debug-adapter-protocol/issues/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 commented 11 months ago

~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 commented 11 months ago

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 commented 8 months ago

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

mfussenegger / nvim-dap