-
Notifications
You must be signed in to change notification settings - Fork 763
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
Add abortSignal to unaryCall #1240
Conversation
|
Hi! Thanks a lot for the interests here and the PR! A few quick thoughts.. :)
Just a few quick thoughts before i forget.. but there are maybe more.. :) Thanks so much for your contrib here again! 😃 (UPDATED with inputs from @stanley-cheung) |
Hello, what's the progress?😊 |
@dbssAlan hi thanks for checking! i think we won't take this PR as is, due to the API change requiring internal validations. Rather, we will:
So, it'll be pending on our side to do Step 1 first. Thanks! |
Hey! Any updates on this? If not, I was wondering what patterns you use to cancel grpc calls via the Promise client. |
I think that we can use |
Hi! I'll try to find some time to experiment with this in the coming weeks. thanks! (i.e. Implement and experiment this feature in Google internal first. as mentioned above) |
I managed to "hack it" by using "AbortController" and verifying if an abort signal has been sent. |
Just curious how exactly did you manage to "hack it"? 😃
Before thie PR i don't think it's possible to cancel an ongoing grpc call so far.. |
You're right. What I meant by "hacked it" was that I added additional code in my services that double checks that request was meant to be cancelled, before doing any side effects. So I'm not actually cancelling it for real. Something like:
|
@iampava aha i see! Thanks for clarifying! 😃 In this case, i'm supposing that you don't have a way to pass the |
@sampajano correct. I have to admit, it would be nice if For me, this was my first thought: let me see if |
In the meanwhile, I'll close this PR for now since this probably won't be the API we'll adopt (we'll probably try to make it more generic and extensible, in case we need to pass more similar options later). Thanks for the contrib! |
@sampajano have any progress ? Can I abort unary call ? export class Client extends GrpcWebImpl {
unary<T extends UnaryMethodDefinitionish>(
methodDesc: T,
_request: any,
metadata: grpc.Metadata | undefined,
): Promise<any> {
const request = { ..._request, ...methodDesc.requestType };
const maybeCombinedMetadata =
metadata && this.options.metadata
? new BrowserHeaders({ ...this.options?.metadata.headersMap, ...metadata?.headersMap })
: metadata || this.options.metadata;
let req: grpc.Request | null = null;
const deffer: any = {};
const promise = new Promise((resolve, reject) => {
deffer.resolve = resolve;
deffer.reject = reject;
req = grpc.unary(methodDesc, {
request,
host: this.host,
metadata: maybeCombinedMetadata,
transport: this.options.transport,
debug: this.options.debug,
onEnd: response => {
if (response.status === grpc.Code.OK) {
resolve(response.message!.toObject());
} else {
const err = new GrpcWebError(response.statusMessage, response.status, response.trailers);
reject(err);
}
},
});
});
// @ts-ignore
promise.abort = () => {
req?.close();
deffer?.reject(
new GrpcWebError(
'CANCELLED',
grpc.Code.Canceled,
new BrowserHeaders({ ...(this.options?.metadata?.headersMap ?? {}), ...metadata?.headersMap }),
),
);
};
return promise;
}
} I tried to create own class extends web grpc code close method from request dont send error when call req.cancel() |
@dmitryshelomanov Hi thanks for checking. This is planned for the upcoming quarter so hopefully i will provide an update soon! |
Any updates on this? |
@maja42 Hi thanks for checking! I intended to do this but was not able to prioritize it yet.. i'll again bump this in my list and i hope to get to it in the upcoming weeks! |
The goal of this PR is to add an
abortSignal
parameter when making calls using thePromiseClient
, which cancels the gRPC stream when the signal is aborted.There has already been interest in being able to cancel calls made using promises: #946