Skip to content
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

handle_space_success is not bound properly in JS client #8695

Closed
1 task done
ericwood73 opened this issue Jul 3, 2024 · 3 comments · Fixed by #8699
Closed
1 task done

handle_space_success is not bound properly in JS client #8695

ericwood73 opened this issue Jul 3, 2024 · 3 comments · Fixed by #8699
Assignees
Labels
bug Something isn't working gradio_client Related to the one of the gradio client libraries

Comments

@ericwood73
Copy link

Describe the bug

In the JS(TS) client, _resolve_config calls check_space_status and passes this.handle_space_success. In handle_space_success an error is thrown if this is not defined. This error will always occur whenever check_space_status is called because this.handle_space_success does not have this bound. To fix this, define this.handle_space_success = this.handle_space_success.bind(this) in the client constructor. This will prevent the error; however, it doesn't appear that status_callbacks will ever work correctly because they depend on being able to reach the space and if the space is paused or sleeping, it appears the _resolve_config will always fail.

A workaround is to create a subclass of Client that properly binds the handle_space_success function:

import { Client as BaseClient, SpaceStatus } from "@gradio/client";

class Client extends BaseClient {
    
    static async connect(spaceId: string | undefined, options: any) {
        return BaseClient.connect(spaceId, options);
    }

    constructor(
        app_reference: string,
		options: any = { events: ["data"] }
    ) {
        super(app_reference, options);
        this.handle_space_success = this.handle_space_success.bind(this);
    }

    async handle_space_success(status: SpaceStatus) {
        return super.handle_space_success(status);
    }
}

Then use this client instead of the gradio client.

Have you searched existing issues? 🔎

  • I have searched and found no existing issues

Reproduction

import { Client , SpaceStatus } from "@gradio/client";
const app = await Client.connect(spaceId, { 
        hf_token: hfToken,
	    status_callback: (space_status: SpaceStatus) => 
                console.log("Status updated: ", space_status), 
        // This is an undocumented option that is apparently required if we have more outputs than inputs
        with_null_state: true,
 })

Pause the HF Space and run this code and you will see the error thrown when handle_space_success is called and this is not defined.

Screenshot

No response

Logs

The error thrown from the Client is `Error: Could not resolve app config. `

System Info

"@gradio/client": "^1.1.1",

Severity

I can work around it

@ericwood73 ericwood73 added the bug Something isn't working label Jul 3, 2024
@pngwn
Copy link
Member

pngwn commented Jul 3, 2024

Good catch, we will fix this.

I think it is expected that a paused space will throw an error because a paused space may never restart (paused space have been manually stopped by the creator), and the client needs to be able to connect to the space to do anything. A sleeping space, however, should eventually resolve because the client should wake it up and then wait for it to move to running. Sleeping spaces have just been inactive for some time and wake up when a request is made.

@pngwn
Copy link
Member

pngwn commented Jul 3, 2024

cc @hannahblair

@pngwn pngwn added the gradio_client Related to the one of the gradio client libraries label Jul 3, 2024
@ericwood73
Copy link
Author

Note that the error thrown when it can't connect to the space is a little misleading:
TypeError: Cannot destructure property 'config' of 'undefined' as it is undefined.. This appears to be caused by this code in init:

await this._resolve_config().then(
        ({ config }) => this._resolve_hearbeat(config)
      );

this._resolve_config throws an error if it can't connect to the space, but the error isn't handled so then is called with an undefined value which fails to destructure. Should maybe do

const { config } = await this._resolve_config();
this._resolve_heartbeat(config)

instead so that the catch handler will catch the rejection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working gradio_client Related to the one of the gradio client libraries
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants