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

Document error behavior for HTTP & OpenAPI #985

Open
matthias-pichler opened this issue Aug 21, 2024 Discussed in #978 · 4 comments
Open

Document error behavior for HTTP & OpenAPI #985

matthias-pichler opened this issue Aug 21, 2024 Discussed in #978 · 4 comments
Assignees
Labels
change: documentation Improvements or additions to documentation. It won't impact a version change. change: fix Something isn't working. Impacts in a minor version change.
Milestone

Comments

@matthias-pichler
Copy link
Collaborator

Discussed in #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]

@cdavernas
Copy link
Member

cdavernas commented Aug 21, 2024

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 cdavernas added change: fix Something isn't working. Impacts in a minor version change. change: documentation Improvements or additions to documentation. It won't impact a version change. labels Aug 21, 2024
@cdavernas cdavernas added this to the v1.0.0 milestone Aug 21, 2024
@ricardozanini
Copy link
Member

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.

@cdavernas
Copy link
Member

cdavernas commented Aug 28, 2024

@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.

@ricardozanini
Copy link
Member

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.

@cdavernas cdavernas modified the milestones: v1.0.0-alpha3, v1.0.0 Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change: documentation Improvements or additions to documentation. It won't impact a version change. change: fix Something isn't working. Impacts in a minor version change.
Projects
Status: Backlog
Development

No branches or pull requests

3 participants