Azure / azure-functions-durable-js

JavaScript library for using the Durable Functions bindings
https://www.npmjs.com/package/durable-functions
MIT License
128 stars 46 forks source link

Update `client.getStatus` to be more consistent with type contract and more type-safe #512

Closed hossam-nasr closed 1 year ago

hossam-nasr commented 1 year ago

Resolves #486 , see issue for more details. In addition to the issue outlined in #486, this PR also updates the behavior of client.getStatus() as follows:

Scenario Old behavior New behavior
Happy scenario: extension responds with 200 or 202, but the timestamps are strings Object is passed as-is to the customer, with timestamps as stings, breaking type contract User is passed a valid DurableOrchestrationStatus object with timestamps as Date objects
Invalid or not found instanceId, extension responds with 404 and empty body User is passed an empty string, breaking type contract (#486) Exception is thrown
Extension encounters an unhandled exception, responds with 500 User is passed the response body containing exception information as the status object, breaking type contract Exception is thrown
Extension responds with valid response code, but some fields are missing (e.g., missing name property) Object is passed as-is to the user, breaking type contract Exception is thrown
Extension responds with valid response code, but empty response body User is passed an empty string, breaking type contract Exception is thrown

In addition to these behavior changes, this PR also:

P.S.: This is a breaking change

davidmrdavid commented 1 year ago

@hossam-nasr:

I'm curious to understand this scenario from your PR description a bit more:

""" Extension responds with valid response code, but some fields are missing (e.g., missing name property) """

Is this a valid response you've seen from the extension, or just a hypothetical scenario based on the previous implementation? If it's a response you've observed, what triggers it?

hossam-nasr commented 1 year ago

""" Extension responds with valid response code, but some fields are missing (e.g., missing name property) """

Is this a valid response you've seen from the extension, or just a hypothetical scenario based on the previous implementation? If it's a response you've observed, what triggers it?

This isn't a valid response and shouldn't be expected in regular behavior, but this is a safeguard I've added to protect us from similar situations as #486 just in case