specification icon indicating copy to clipboard operation
specification copied to clipboard

Document error behavior for HTTP & OpenAPI

Open matthias-pichler opened this issue 1 year ago • 4 comments

Discussed in https://github.com/serverlessworkflow/specification/discussions/978

Originally posted by matthias-pichler August 19, 2024 Currently the default output behavior for the call: http task is content which returns the content (i.e. body) of the response. This is very convenient since authors most likely want the response data. However it is very unintuitive when the API returns an error. Since jq will happily return null if non existing properties are accessed subsequent tasks might seem to work or the workflow might fail many steps downstream. Should we update the ergonomics around error handling?

  1. Should call: http raise an error if a not-ok (outside of [200, 399]) status is reported?
  2. Should the default output type be changed to response to report the whole response?
  3. should we allow to specify an expected status (range)?

We should document that call: http and call: openapi raise a communication error for status codes outside of [200, 399]

matthias-pichler avatar Aug 21 '24 12:08 matthias-pichler

As suggested by you in #978, I think we should add a new redirects boolean property, defaulting to false.

As a matter of consequence, when it has been set to true, http calls are successfull when status is in [200,399] range. When set to false, they are considered successfull only when in [200,299] range.

WDYT?


If applicable, then we should decide the name of said property. I propose:

  1. redirect(s)
  2. redirection(s)
  3. allowRedirect(s)
  4. supportRedirect(s)
  5. acceptRedirect(s) ...

I have a strong preference for either 1 or 2, as they are single words.

cdavernas avatar Aug 21 '24 12:08 cdavernas

Shouldn't the allow-redirects be handled by the underlying platform? The HTTP connectors can configure if it will follow redirects automatically or not. Hence, implementors will have to bind this option to the underlying HTTP connector. I'm unsure if we should add this option to the DSL since it's an infrastructure configuration.

ricardozanini avatar Aug 27 '24 13:08 ricardozanini

@ricardozanini This is IMHO definitly related to the DSL, as in some cases, an author might/have to explicitly authorize a redirect, and she might choose/have to to explicitly forbid it in others.

Delegating that to runtimes would restrict authors choice/use cases. Additionally, workflows might have completly different behaviors on implementations with different redirection strategies, which would drastically hinder portability.

cdavernas avatar Aug 28 '24 12:08 cdavernas

My point is that any HTTP library will set the follow-redirects property to true independently of the implementation. Do you have a use case where the DSL author should set this property to the workflow level?

Bringing this to the DSL will enforce implementations to bind the DSL to an infrastructure configuration.

ricardozanini avatar Aug 28 '24 16:08 ricardozanini

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Nov 22 '24 00:11 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jan 07 '25 00:01 github-actions[bot]

Do you have a use case where the DSL author should set this property to the workflow level?

@ricardozanini Sorry, I had overlook you last comment. There are a lot of use cases, yes. The implementation alone cannot decide whether or not to consider a redirect as an error, it depends on too many operation-specific variables.

cdavernas avatar Jan 09 '25 16:01 cdavernas