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

feat #22: New invoke syntax proposal #23

Closed
wants to merge 4 commits into from

Conversation

vejja
Copy link

@vejja vejja commented Jul 11, 2022

What kind of change does this PR introduce?

Closes #22

What is the current behavior?

The invoke() function currently implements the following behavior:

What is the new behavior?

The new behavior modifies the prototype of invoke() in accordance with the proposal set out under feature request #22 :

  • The input parameter can be provided as an object. The invoke function automagically serializes the body and sets the headers. This reduces the amount of boilerplate for the most common input data types
  • The output is still de-serialized automagically. However the richer underlying Response object can now be accessed via a third options parameter, which provides callback entry points to the fetch() call.

Please refer to the accompanying README.md file for further details on the new proposed behavior of invoke.

Additional context

This PR closes #22.
It also fixes #20 and fixes #14.

Notes:

  1. This PR is a draft which is intended to be reviewed as a skeleton-basis only for comments and feedbacks.
  2. This PR makes extensive use of the standard Fetch, Request, and Response Web APIs. Previous options that used to be passed as named parameters are now passed according to the prototypes of these APIs.
  3. The outgoing request headers are written according to the data type on the way out, and the incoming data type is inferred from reading the response headers on the way back. This is an opinionated convention which I believe covers 99.9% of the use cases of a serverless fetch() functionality.

vejja added 4 commits July 1, 2022 12:32
invoke() call throws if shouldThrowOnError flag is set
- rename HttpError to FunctionsError
- pull out status & statusText from FunctionsError
- status and statusText are optional
@vejja vejja marked this pull request as ready for review July 11, 2022 13:15
@inian
Copy link
Member

inian commented Aug 15, 2022

Hey @vejja, thank you for the PR ❤️ We couldn't merge this in directly since this would be a breaking change and was a lot going on here. To break it down, the PR proposes these changes

automatically adding content types to request
automatically parsing objects as json by default
returning an error if function throws an error
All these have been added to our next branch. We will release this in our next breaking version of the library.

request and response transformers
I am not sure if I like this pattern, it mixes callbacks and promises for the user. It also blocks certain transformations (method, etc) which is more things for the developer to remember. It is easier to just add more optional arguments to invokeOptions and let the user transform before or after the function call.

@inian inian closed this Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants