Skip to content

Conversation

@vr-varad
Copy link
Contributor

@vr-varad vr-varad commented Sep 30, 2025

Fixes #151

@vr-varad
Copy link
Contributor Author

The workflow handlers registered are:

  • baseworkflowhandle
  • workflowhandle
  • workflowPollingHandle

Now I need to make changes in the tests? // @maxdml

@maxdml
Copy link
Collaborator

maxdml commented Sep 30, 2025

Thanks @vr-varad , yes we should have a test (either modifying an existing one or adding a new one) where a workflow and a step would return a WorkflowHandle interface.

I don't think we need to register baseworkflowhandle : the user will always get a workflowhandle or workflowPollingHandle.

@maxdml
Copy link
Collaborator

maxdml commented Oct 2, 2025

@vr-varad also check #166: adding a safeGobRegister helper, which we should use in this PR too.

@vr-varad
Copy link
Contributor Author

vr-varad commented Oct 3, 2025

Does the failing test mean the registration has failed? // @maxdml

@maxdml
Copy link
Collaborator

maxdml commented Oct 6, 2025

Does the failing test mean the registration has failed? // @maxdml

I think there is another, unrelated bug. It looks like we don't check if the concrete value provided to safeGobRegister is nil. Gob does panic if that's the case. I'll be providing a fix soon.

@maxdml
Copy link
Collaborator

maxdml commented Oct 6, 2025

@vr-varad the issue is that we can't automatically register interfaces during RegisterWorkflow. In this case, when you try to register a workflow that returns an interface, behind the hood we create a zero value and attempt to register it. For all zero values, this works fine, but for interfaces, gob requires that the underlying concrete type has been registered first.

There is another issue I now realize: the workflow handles don't export any field, which makes them unsuitable for encoding as well.

In this case let's just cancel this feature, ignore registration of nil interfaces, and ask users to manually register their types.

@maxdml maxdml closed this Oct 6, 2025
@vr-varad
Copy link
Contributor Author

vr-varad commented Oct 6, 2025

Makes Sense @maxdml!!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Register WorkflowHandle for gob encoding automatically.

2 participants