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

WaitForCompletionOrCreateCheckStatusResponseAsync has no way to restore default behavior #1471

Closed
ConnorMcMahon opened this issue Sep 10, 2020 · 2 comments · Fixed by #1572
Closed
Assignees
Labels

Comments

@ConnorMcMahon
Copy link
Contributor

In versions 1.x of the extension, calling the Get Status API would return a 500 if the orchestration instance had failed. To follow better API design, in 2.x of the extension, we changed the default behavior to return a 200 while simply marking the status field as "Failed". On the actual HTTP API and the IDurableClient.CreateCheckStatusResponse API, we have an optional parameter returnInternalServerErrorOnFailure to restore the 1.x behavior.

As @dixonte pointed out, IDurableClient.WaitForCompletionOrCreateCheckStatusResponseAsync is missing this optional parameter, leaving no way to have it return an HTTP response payload with the correct query string parameter filled out.

@ConnorMcMahon
Copy link
Contributor Author

I am hesitant for two reasons:

  1. API bloat. Adding optional parameters adds both a maintainability burden as well as potentially confuses API consumers who don't need/want that functionality.
  2. Adding optional parameters breaks peoples tests for Moq.

@dixonte
Copy link
Contributor

dixonte commented Sep 10, 2020

If this feature is added -- and possibly regardless -- I would like to suggest that in the case where IDurableClient.WaitForCompletionOrCreateCheckStatusResponseAsync returns before creating the check status response, the response should follow the same format as the Get Status API. This would allow clients to reliably check the Status regardless of how quickly the orchestration completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants