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: support async custom functions #1058

Merged
merged 11 commits into from
Apr 16, 2020
Merged

feat: support async custom functions #1058

merged 11 commits into from
Apr 16, 2020

Conversation

P0lip
Copy link
Contributor

@P0lip P0lip commented Apr 5, 2020

Closes #694

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

@P0lip P0lip added enhancement New feature or request WIP labels Apr 5, 2020
@P0lip P0lip self-assigned this Apr 5, 2020
@P0lip P0lip force-pushed the feat/async-fns branch 2 times, most recently from be99726 to 19ad179 Compare April 7, 2020 23:25
src/runner.ts Outdated Show resolved Hide resolved
@philsturgeon
Copy link
Contributor

philsturgeon commented Apr 10, 2020 via email

@P0lip P0lip marked this pull request as ready for review April 14, 2020 14:34
@P0lip P0lip marked this pull request as ready for review April 14, 2020 14:34
@P0lip P0lip removed the WIP label Apr 14, 2020
@P0lip P0lip requested review from philsturgeon and nulltoken April 14, 2020 14:34
if (res.ok) {
this.cache.set(CACHE_KEY, await res.json());
} else {
// you can either re-try or just throw an error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@P0lip What's the effect from throwing from within a function? Does the whole process crash down? Do we get a special kind of result or a console log?

From the function writer standpoint, what kind of code should be written

  • if I don't want this failure being unnoticed
  • if I prefer to see a CI build fail rather than having it pass for the wrong reasons

Note: I know this is kind off-topic with regards to the scope of the PR but... well... this is documentation and, to be honest, the doco actually lacks guidance about this.

Note 2: If you prefer this to be dealt with later, could you please log an issue to keep track of that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@P0lip What's the effect from throwing from within a function? Does the whole process crash down? Do we get a special kind of result or a console log?

We'd get a console.log, same as we do when any other custom function throws an exception.
The whole process would still keep on going.

if I prefer to see a CI build fail rather than having it pass for the wrong reasons

I'd return a regular error, i.e.

return [{ message: 'Network error. The check could not be performed' }]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@P0lip That's very valuable info. I've logged #1096 as I think this deserves a doco section of its own.

@philsturgeon
Copy link
Contributor

This is so great!

@P0lip P0lip merged commit 367bb40 into develop Apr 16, 2020
@P0lip P0lip deleted the feat/async-fns branch April 16, 2020 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for async custom functions?
3 participants