jdiaz5513 / capnp-ts Goto Github PK

So I discovered this purely by luck.
When I run capnpc-ts it runs into this error:

Error: CAPNP-TS028 Attempted to convert CodeGeneratorRequest_Struct_Pointer_0@0x00000000,[00 00 00 00 00 00 02 00],limit:0x7fffffff to a struct with the wrong pointer size (found: 3).
    at CodeGeneratorRequest.Pointer._validate (/Users/efokschaner/dev/efokschaner/capnp-ts/packages/capnp-ts/lib/serialization/pointers/pointer.js:539:35)

From

So I've figured out why:
A few weeks ago when starting work on this I ran brew install capnp. At that time I received capnp v0.5.3. The latest version which came out in the last few months is v0.6.0. Between those versions a capnpVersion member was added to CodeGeneratorRequest. Our _validate check fails because I'm feeding in an object with 2 pointers when the schema described 3.

Now the capnproto docs don't state anywhere (as far as i could see) what the correct behaviour should be under exactly this circumstance. There's stuff about default values, and there's even some good discussion about what should happen when a "client fails to set a particular field" (see https://capnproto.org/faq.html#how-do-i-make-a-field-required-like-in-protocol-buffers) where you could imagine "Failing to set" being the same as "Used an older schema and therefore didn't set".

If I understand correctly, one of capnproto's design goals is to only perform validation once its needed to access so that "you only pay for what you use". Another goal is to provide flexible yet predictable backwards compatibility, either when newer messages are interpreted via older schemas, OR when older messages are interpreted via newer schemas.

I'm still digging around the C++ version to see what the implementation-defined specification is for this scenario.

So far, it seems to me that the right interpretation is that on the receiving side, deserialization is valid and it should be as if the value was the default (either the implicit or explicit default if an explicit default was made for the field). Let me know if you agree / disagree, or if you can find any more concrete information about this scenario.

I also want to ask if you're aware of anywhere else in the library currently that might also be too strict or eager in its validation like this?

So I'll probably unblock myself for now by upgrading to capnp 0.6.0, but this has given me some good ideas for some end to end tests around producers and consumers with different (but compatible) schemas on the next iteration :)

Hi!

Very happy to see that imports have been fixed in the latest release.
I ran into an issue trying to compile a schema that includes https://github.com/capnproto/capnproto/blob/master/c%2B%2B/src/capnp/c%2B%2B.capnp

Here's an example file:

using Cxx = import "c++.capnp";
$Cxx.namespace("Initrode");

struct Foo {
  bar @0 :Text;
}

which returns Error: CAPNPC-TS002 Failed to look up node id Infinity. when compiling.

Underlying issue was with gulp-typescript and already patched; adding this issue here as a reminder to upgrade once an update hits npm.

ivogabe/gulp-typescript#527

One possible way to do it is by only defining public API methods on Pointer, Struct, List, etc. and converting the methods we want to hide into module-level methods.

For example:

// From this:
class Pointer {

  _validate() {

    if (this.segment === undefined) throw new Error();

  }

}

// To this:
function validatePointer(ptr: Pointer) {

  if (ptr.segment === undefined) throw new Error();

}

I think there's not a lot of method overriding going on, so maybe that's the cleanest approach.

The schema compiler (capnpc-ts) currently has all the code necessary to transpile the emitted .ts file into .js using the TypeScript compiler API directly, but I have failed to get it to actually work without choking on the capnp-ts import.

The compiler configuration is here:

The transpile step is commented out here:

I tried using the baseUrl and paths options to no avail; but it's entirely possible I missed something.

Ideally, the TypeScript compiler should load capnpc-ts's copy of capnp-ts from node_modules, and not depend on capnp-ts being available from the path it's compiling from... or something.

After updating capnpc-js to v0.2.3, I see that capnpc-js/src/index.ts has

  noUnusedLocals: false,
  noUnusedParameters: false,

but capnpc-js/lib/index.js still has

        noUnusedLocals: true,
        noUnusedParameters: true,

which causes #83 to continue happening.

Having updated to 0.2.2 with the c++.capnp fix, I realized I'm actually having issues generating JS files for even the most basic of schemas.
Trying to compile bar.capnp from #75 yields:

bar.capnp.ts:7:13 'capnp' is declared but its value is never read.
bar.capnp.ts:7:13 'capnp' is declared but its value is never read.
Error: CAPNPC-TS008 Failed to transpile emitted schema source code; see above for error messages.

Is there some extra TS code being generated after the refactor that's getting caught in the transpile step?

Running gulp coverage definitely does the wrong thing; it will make separate test runs for each package then generate the coverage report for the last package that was run.

That task needs some deep rethinking. The way node-tap is supposed to work for coverage is you run it once with nyc enabled, then again to generate the actual coverage report. The only way to make this work across multiple packages is to combine the tests into a single run (so gather all the spec files from capnp-ts and capnpc-ts and run them together) before generating the coverage report.

Additionally, it looks like there's additional trouble with the source maps generated by gulp-sourcemaps. They don't set sourceRoot so nyc doesn't know how to reference the original source. See istanbuljs/nyc#359 (comment).

The watcher is supposed to ignore failures and keep running, but the command bails completely if a test run fails.

When hiding implementation details in accessible places (and especially a dynamic language) it definitely pays to use symbols that have low accidental collision likelihood.

Take our static member called _id for example (https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/std/schema.capnp.ts#L149). I'm sure there's some other library out there somewhere that decided they'd hide _id members on class (constructor) objects, for us to collide with.

I noticed the official C++ compiler does the following: https://github.com/capnproto/capnproto/blob/master/c%2B%2B/src/capnp/schema.capnp.h#L95

Its nested bundle has two nice features IMO. 1. It's got a very project / tool specific name, so other systems are less likely to conflict. 2. Even if autocomplete / intellisense / whatever is aware of the implementation detail members it sucks them into a single ignorable entry in the list.

Therefore, I propose we take a similar approach with the implementation details that we're bundling into types.

Hi,

I am trying to install the Js serialization and schema generation tools for capnp but none of the installation commands given in the READme work.

ERROR:
npm ERR! code E404
npm ERR! 404 Not Found: capnpc-ts@latest

npm ERR! A complete log of this run can be found in:

Steps to reproduce:
npm install --save capnpc-js
or
npm install --save capnpc-ts
or
npm install --save capnp-ts.

If there is any other way of installing it, please let me know. Also, I am primarily looking for JS implementation of compiling schema and deserializing the messages.

Any help would be appreciated.

Hi!

Thanks so much for fixing the previous int64 issue so quickly. I believe I ran into another bug trying to actually write these int64 values to the capnp message.

For example, writing capnp.Uint64.fromNumber(Math.pow(500000, 2)) into a Uint64 field and reading it out returns 891896832 (instead of 250000000000 which is still well under the max value).

I investigated this a little bit and it seems like https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/serialization/segment.ts#L451 could be the culprit - val.buffer is actually a Uint8 array of length 8 but only the first half of it seems to be getting set. Replacing this with e.g.

for (var i = 0; i < val.buffer.length; ++i) {
  this._dv.setUint8(byteOffset + i, val.buffer[i]);
}

seemed to do the trick and fix the issue I was seeing. https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/serialization/segment.ts#L405 seems to have a similar issue although I haven't personally verified that one yet.

Let me know if you have any questions or other suggestions to work around this.

This started with me thinking that I frequently have questions that i don't feel justified spamming up the issues system with. I was about to suggest we open some alternative discussion channel.
But then I realised we'd probably need to sub-categorise things properly anyway unless we had as many systems for discussion as there were types of things that can come up therefore I'm leaning towards saying let's do issues but with better tagging.

Here are some obvious category suggestions just to get started:

• Bug reports
• Todo's
• Support requests (can be used for both external and internal development help)
• Suggestions

capnpc -onode_modules/capnpc-js/bin/capnpc-js.js addressbook.capnp

RangeError: Offset is outside the bounds of the DataView
at DataView.getInt32 (native)
at Segment.getInt32 (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnp-ts/lib/serialization/segment.js:132:29)
at Pointer._getOffsetWords (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnp-ts/lib/serialization/pointers/pointer.js:326:33)
at class_1.Pointer._getTargetListLength (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnp-ts/lib/serialization/pointers/pointer.js:405:58)
at class_1.List.getLength (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnp-ts/lib/serialization/pointers/list.js:69:25)
at class_1.List.map (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnp-ts/lib/serialization/pointers/list.js:72:31)
at generateStructNode (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnpc-ts/lib/generators.js:323:49)
at generateNode (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnpc-ts/lib/generators.js:105:17)
at /Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnpc-ts/lib/generators.js:101:50
at Array.forEach (native)
at Object.generateNode (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnpc-ts/lib/generators.js:101:20)
at /Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnpc-ts/lib/compiler.js:24:158
at Array.forEach (native)
at compile (/Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnpc-ts/lib/compiler.js:24:115)
at /Users/pooja/Desktop/ulema_backup/capnp-ts_implementation/node_modules/capnpc-ts/lib/compiler.js:48:40
at Array.forEach (native)
node_modules/capnpc-js/bin/capnpc-js.js: plugin failed: exit code 1

Right now, unpack runs on the entire packed ArrayBuffer and combines all the message's segments into a single ArrayBuffer, including the stream frame header.

This is silly, given that it's going to need to be split up again into separate buffers soon afterward in Message.getFramedSegments(). A (huge?) number of CPU cycles can be saved by enabling the unpack algorithm to receive byte offset and expected length arguments. Once the expected number of words have been unpacked the unpack function should return what it got up to that point, including an offset to where it stopped unpacking. That'll also eliminate the need for getUnpackedByteLength() while unpacking since the stream frame header will dictate the length of each segment.

To avoid multiple return values in unpack() (ick!) it's probably sane to pass a destination ArrayBuffer that's already allocated to the right size, and use that for the expected length. It makes it harder to just unpack an arbitrary buffer of bytes with unknown final size, but will that ever be needed?

I'm introducing capnpc-js as a tracing identifier, so it would seem natural to make the other one match its package name.

Tests should follow the same general pattern as capnp-ts. Best to start with some basic integration level tests to get most of the coverage out of the way, then drill down with unit specs to hit difficult branches.

I noticed that in our codegen we transform nested things into top-level members with an underscore between parent and nested member. eg. CodeGeneratorRequest_RequestedFile Whereas (for example) in the official capnproto C++ compiler they generate an actually nested RequestedFile struct inside the CodeGeneratorRequest struct.

See:
https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/std/schema.capnp.ts#L989
Vs:
https://github.com/capnproto/capnproto/blob/1dcabbf81d772069119773025d44c29df19ccd30/c%2B%2B/src/capnp/schema.capnp.h#L626

I think it would be more intuitive to match the C++ style and nest emitted things when they're nested in the schema, if there's no barrier to doing so in TS that I'm unaware of.

Wondering if we can split up some of the work? Maybe with the schema compiler, for example? Let me know if you'd like help with that or anything else.

Hi @jdiaz5513 ,

Thanks for the npm publish. Your documentation mentions that the the schema.capnp.js gets generated at the same path as the input schema.capnp. The default behavior of capnp compile allows the setting of an output path too. Is there a way I can avail that functionality here? I tried the following command:

capnp compile -o<path-to-capnpc-js.js>: <path-to-schema.capnp>

Note: running capnpc-js throws error if an absolute path is given. You have to specify /bin/capnpc-js.js

Let me know if you have any imputs.

I found node-tap is quite minor and it doesn't integrate well with Gulp. Are you using any exclusive features from it? If so, feel free to close. If not, I would recommend switching to Jasmine.

I would like to do the migration for you too, if you want to focus on other things.

Didn't see anything mentioning imports (or lack of support for them) in the README, so just wanted to document the behavior I'm seeing.

I created bar.capnp as follows:

struct Baz {
  bar @0 :Text;
}

I then tried to use this Baz message type from foo.capnp. Here are some of the attempts (based on the three variants listed at https://capnproto.org/language.html):

using Bar = import "bar.capnp";
struct Foo {
  baz @0 :import "bar.capnp".Baz;
}

using Bar = import "bar.capnp";
struct Foo {
  baz @0 :Bar.Baz;
}

using import "bar.capnp".Baz;
struct Foo {
  baz @0 :Baz;
}

For either of these three versions, capnpc -o js foo.capnp fails with foo.capnp.ts:13:34 Cannot find name 'Baz'. errors. Best I can tell, the generated .ts files don't map the import statements to any includes of the .capnp.ts files and so compiling that to JS fails.

Is there a working version of imports available now? If not, is it on the roadmap? Thanks!

gulp v4's watch function has an option for exactly this, could be a great reason to switch to v4 instead.

Starting from a fresh repo, I followed the instructions under "Building" in the readme, but they didn't quite work. The build failed as shown below.

I found that I was able to fix this problem by symlinking the directories under packages into the node_modules directory, in particular so that capnp-ts successfully resolved, but I presume that's not the "right" solution. Note that I haven't used Yarn, Gulp, or Typescript before, but have used npm and node plenty.

kenton@zero:~/src/capnp-ts:master$ yarn install
yarn install v0.27.5
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
Done in 2.57s.
kenton@zero:~/src/capnp-ts:master$ npm run build

> [email protected] build /home/kenton/src/capnp-ts
> gulp build

[12:52:17] Using gulpfile ~/src/capnp-ts/gulpfile.js
[12:52:17] Starting 'build:capnp-ts:lib'...
[12:52:19] Finished 'build:capnp-ts:lib' after 2.59 s
[12:52:19] Starting 'build:capnpc-ts:lib'...

5 import * as s from 'capnp-ts/lib/std/schema.capnp';
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

packages/capnpc-ts/src/ast-creators.ts(5,20): error TS2307: Cannot find module 'capnp-ts/lib/std/schema.capnp'.


6 import {format, pad} from 'capnp-ts/lib/util';
                            ~~~~~~~~~~~~~~~~~~~

packages/capnpc-ts/src/ast-creators.ts(6,27): error TS2307: Cannot find module 'capnp-ts/lib/util'.


5 import * as s from 'capnp-ts/lib/std/schema.capnp';
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

packages/capnpc-ts/src/code-generator-file-context.ts(5,20): error TS2307: Cannot find module 'capnp-ts/lib/std/schema.capnp'.

... more similar errors follow ...

There seems to be an issue with the two's complement implementation of the Int64 field type (or how I am using it).

Storing some value between 128 and 255 (say, 130) in a Uint64 works as expected but storing this in an Int64 returns a large negative number.
https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/types/int64.ts#L197 seems to be negating the value based on the most significant bit of the least significant byte (which is 1 for values between 128 and 255) - is this a bug?

I was curious about this idea in the readme:

NOTE: At some point in the future the capnpc-ts package may build the capnpc binary as a native dependency, making this step somewhat unnecessary.

Given that serialisation / rpc protocols are often used to talk across technology stacks I suspect that most users of this would (at least eventually) need to have a capnp setup independent of the stuff we're providing here. Therefore I think it's a bit odd/niche to provide such a utility.

Are any other languages / compiler plugins doing such things?
What are the motivations for doing this?

In the C++ implementation, any time a pointer is overwritten making the previously pointed-to object unreachable, said object is recursively overwritten with zeros.

This is important for two reasons:

• This situation unfortunately creates a "hole" in the message -- data that wastes space on the wire, but which isn't actually part of the message. There's no reasonable way to avoid a hole, but zeroing it out ensures that the hole compresses well, if any compression is applied.
• Application developers may not expect that data they thought they "removed" from the message may in fact still be present on the wire. This can be a security problem, if the data was intentionally removed to hide it from the eventual recipient. We have in fact seen real-world use cases where this was a security problem.

For these reasons I recommend implementing the C++ behavior in capnp-ts.

Note that orphans are tricky here. In C++, we have destructors, so if an orphan is destroyed without being adopted, we can automatically go zero out the object at that time. In Javascript you'll need an explicit dispose() method, I suppose. (Or if you are feeling evil, you could call it euthanize()...)

Hi!

I've got a question about how capnp-ts deals with fields being added to the underlying capnp schema.
I had a struct with two fields (a text field and a pointer to another struct), which was part of a larger message that I serialized into bytes using the capnp-ts library.
I then added two more (text) fields at the end of this struct (with tags 2 and 3), regenerated the .capnp.js implementations, and tried to read these new fields from the previously serialized bytes.

The result was
Error: CAPNP-TS024 Attempted to convert pointer Pointer_1@0x00000020,[f4 ff ff ff 00 00 04 00],limit:0x7ffffffa to a 1 type.
and digging into it a little bit more it seemed like https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/serialization/pointers/pointer.ts#L895 was getting a pointer type of 0 when it was expecting a pointer type of 1.

I see that https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/serialization/pointers/struct.ts#L603 acknowledges the struct size mismatch might happen, but to the best of my understanding the "hole" of zeroes in the message also zeroes out the pointer types which causes validation to fail if any other pointer type is expected.

Please let me know if there's any more information that would be helpful here or if there's a better way to deal with this situation.

Thanks!

This will allow bringing node 8 back into the automated build.

Makefiles are hard to understand/write (sorry, it's true), and they're UNIX only.

Are we sure we want to cache all the node_modules folders?
Builds would be potentially better isolated and still well cached if we only cached the yarn cache itself ($HOME/.cache/yarn) see: https://docs.travis-ci.com/user/caching/#yarn-cache
I could see packages in node_modules from previous runs potentially creating build weirdness (lack of idempotence).

If we decide we DO want to cache all node_modules folders, then I wonder if directories supports patterns, it wasn't clear from docs.