feat: implementation of useAbortSignal option for grpc-web

[hersentino]

#731 introduces the useAbortSignal option but it's currently not implemented for grpc-web.

This PR add useAbortSignal for grpc-web, fully working with unary/invoke

[hersentino]

I'm trying to figure out why build (16.x) is failing

[stephenh]

Hi @hersentino , thanks for the PR! I think this looks great!

I'm trying to figure out why build (16.x) is failing

Ah yeah, sorry for not replying sooner but oddly enough CircleCL will cancel all jobs once the first one fails, so a lot of times the issue will look (say) specific to the 16.x build, but really it just happened to fail first, and the issue does actually exist in all versions of node.

I think your changes look good; as a regression test, can you find an example integration/* directory that is using grpc-web and add the useAbortSignal=true method to its parameters.txt? After running yarn bin2ts, you should see the generated code from your changes, which we include in PRs + as part of CI. Thanks!

[stephenh]

Fwiw @hersentino I think landing this PR is still great; but when I went to google "improbable-eng abort signal" to sanity check how it works, and I came across nice-grpc-web:

https://www.npmjs.com/package/nice-grpc-web

Which is a sister project/built on top of ts-proto.

I was scanning their impl and nothing stuck out as me as something we should borrow/replicate for your PR:

https://github.com/deeplay-io/nice-grpc/blob/master/packages/nice-grpc-web/src/client/createUnaryMethod.ts

Which is cool, we can ship your PR as-is, but just thought I'd mention it, since they are actively using ts-proto + grpc-web, it might be worth checking out.

[hersentino]

can you find an example integration/* directory that is using grpc-web and add the useAbortSignal=true method to its parameters.txt ?

I chose integretion/grpc-web-no-streaming-observable because it contains unary method. But might it be better if I create a new example directory just for useAbortSignal option ? It could test the 3 methods createPromiseUnaryMethod, createObservableUnaryMethod and createInvokeMethod

[hersentino]

since they are actively using ts-proto + grpc-web, it might be worth checking out.

It seems to be handle here execute.

They don't use the close method from the client. I'm trying to understand how it works without, I don't a have lot a time right now but I should test locally nice-grpc-web to fully understand the behavior

[hersentino]

I'm sorry @stephenh I forgot to ping you

[stephenh]

@hersentino this seems fine, but it's probably a breaking API change to current codes that are passing foo.UserSettings(request, metadata)?

Can we push the abortSignal last so that it's not a breaking change?

[hersentino]

@stephenh Yep no problem :)

I initially put it in second argument because it was set as 2nd argument in this PR

So I had to also push abortSignal to last argument for the function generateService inside src/generate-services.ts. Not sur if this modification break something somewhere else.

[stephenh]

@hersentino looks great! Thanks!

[stephenh]

🎉 This PR is included in version 1.140.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release
• v1.140.0

Your semantic-release bot 📦🚀