Skip to content

Allow null response#1025

Merged
stanley-cheung merged 1 commit intogrpc:masterfrom
pro-wh:pro-wh/feature/nullish
Jan 23, 2021
Merged

Allow null response#1025
stanley-cheung merged 1 commit intogrpc:masterfrom
pro-wh:pro-wh/feature/nullish

Conversation

@pro-wh
Copy link
Copy Markdown
Contributor

@pro-wh pro-wh commented Jan 22, 2021

We're using a custom codec, where null or undefined responses can be represented, and we use that for void methods, similar to how protobuf uses Empty.

This change allows grpc-web to accept responses that decode to null or undefined.

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jan 22, 2021 •
edited
Loading

CLA Signed

The committers are authorized under a signed CLA.

@pro-wh
Copy link
Copy Markdown
Contributor Author

pro-wh commented Jan 22, 2021

I couldn't build this locally:

$ npm run build

> [email protected] build /home/wh0/work/ekiden/grpc-web/packages/grpc-web
> node scripts/build.js

/home/wh0/work/ekiden/grpc-web/packages/grpc-web/node_modules/.bin/google-closure-compiler --js=exports.js --js=../../javascript --js=../../third_party/closure-library --entry_point=grpc.web.Exports --externs=externs.js --dependency_mode=PRUNE --compilation_level=ADVANCED_OPTIMIZATIONS --generate_exports --export_local_property_definitions --js_output_file=index.js
../../javascript/net/grpc/web/clientreadablestream.js:29: ERROR - [JSC_UNDEFINED_VARIABLE] variable goog is undeclared
goog.module('grpc.web.ClientReadableStream');
^^^^

1 error(s), 0 warning(s)

@pro-wh
Copy link
Copy Markdown
Contributor Author

pro-wh commented Jan 22, 2021

EasyCLA won't accept my CLA for this. It only seems to apply it to another PR I opened, #1026

@pro-wh
Copy link
Copy Markdown
Contributor Author

pro-wh commented Jan 22, 2021

they said I should close and reopen to get the bot to notice the CLA. doing that now

@pro-wh pro-wh closed this Jan 22, 2021
@pro-wh pro-wh reopened this Jan 22, 2021
stanley-cheung
stanley-cheung reviewed Jan 23, 2021
View reviewed changes
Comment thread javascript/net/grpc/web/grpcwebclientbase.js Outdated
@stanley-cheung stanley-cheung merged commit 201c689 into grpc:master Jan 23, 2021
@pro-wh pro-wh deleted the pro-wh/feature/nullish branch January 25, 2021 18:18
@pro-wh pro-wh mentioned this pull request Feb 17, 2021
Closed
@pro-wh pro-wh mentioned this pull request Apr 27, 2022
Merged
@pro-wh
Copy link
Copy Markdown
Contributor Author

pro-wh commented Apr 27, 2022

this turned out not to solve the problem. there are no data events for these payloads at all. opening #1230 as a proposal to add data events for falsy responses

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stanley-cheung stanley-cheung stanley-cheung approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pro-wh @stanley-cheung @grpc-kokoro