fix: prevent server crash on bad function call arguments (closes #64)#149
Open
Rohitbhise0004 wants to merge 1 commit intometacall:masterfrom
Open
fix: prevent server crash on bad function call arguments (closes #64)#149Rohitbhise0004 wants to merge 1 commit intometacall:masterfrom
Rohitbhise0004 wants to merge 1 commit intometacall:masterfrom
Conversation
…call#64) When a MetaCall function is invoked with wrong arguments (e.g. passing a positional arg to a zero-arg Python function), the native runtime threw an exception that was previously unhandled, causing the worker child process to exit. This left any pending HTTP requests hanging forever and made the server appear to go down. Three changes to fully resolve this: 1. worker/protocol.ts: Add a dedicated InvokeError message type so the worker can signal a function-call failure to the master process without reusing InvokeResult with an embedded error field. 2. worker/index.ts: In the Invoke handler catch block, send InvokeError (with the error string) instead of a fake InvokeResult with { error: ... }. This gives the master a clean signal to call eject() on the pending invocation. 3. utils/deploy.ts: - Handle the new InvokeError message type in the master process message handler by calling invoke.reject() so the HTTP client receives an HTTP 500 with the error message instead of hanging. - Fix the exit handler: drain all pending invocations from the invokeQueue (calling eject() on each) so that requests in flight when the worker crashes are never left unresolved. - Guard deployReject / deployResolve with an isDeploySettled flag so the already-resolved deploy promise is never rejected a second time when the worker exits after a successful deployment. - Add null-guard on invokeQueue.get() return value. 4. utils/invoke.ts: Add pendingIds() method to InvokeQueue so the exit handler in deploy.ts can enumerate and drain all pending invocation callbacks. 5. test/test.sh: Add a regression test in est_python_base_app that POSTs a spurious argument to the zero-arg umber() function, asserts HTTP 500 is returned (not a hang), and confirms the FaaS readiness endpoint still responds with 200 afterwards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a MetaCall function is called with the wrong number of arguments (e.g.
curl ... -X POST --data '1'against a zero-argument Python function), the native MetaCall runtime throws an exception inside the worker child process. This unhandled error caused the worker process to exit, which:metacall-deploy --inspect --devwould exit)Fixes Bug: The local server goes down with bad requests #64.
Root Cause
The
proc.on('exit')handler insrc/utils/deploy.tshad a// TODOcomment acknowledging it could not properly reject in-flight invocations when the worker exited mid-call. Pending invocations stored ininvokeQueuewere never resolved or rejected, so their HTTP responses were never sent.Additionally, there was no dedicated error message type for function invocation failures --- the worker reused
InvokeResultwith a{ error: ... }field, which the master process always treated as a success by callingresolve().Changes
src/worker/protocol.tsInvokeErrormessage type to the protocol enum.src/worker/index.tsInvokehandler catch block, sendInvokeError(with the stringified error) instead of a fakeInvokeResultwith an embeddederrorfield.src/utils/invoke.tspendingIds()method toInvokeQueueso callers can enumerate and drain all in-flight invocations.src/utils/deploy.tsInvokeErrormessage type by callinginvoke.reject()so the client receives an HTTP 500 with the error message.exithandler: iterateinvokeQueue.pendingIds()and reject every pending invocation --- resolves the// TODOand prevents requests from hanging when the worker crashes.deployReject/deployResolvewith anisDeploySettledflag to prevent calling them after the deploy promise has already settled.invokeQueue.get()return values.test/test.shtest_python_base_appthat POSTs a spurious argument to the zero-argnumber()function, asserts HTTP 500 is returned, and confirms readiness still returns 200.Before / After
deployRejectcalled on settled promise (silent error)isDeploySettledflag