Skip to content
This repository has been archived by the owner on Jul 12, 2019. It is now read-only.

fix null error handling #117

Merged
merged 1 commit into from
Oct 10, 2018
Merged

fix null error handling #117

merged 1 commit into from
Oct 10, 2018

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Oct 5, 2018

Fixes https://github.com/zapier/zapier/issues/18163.

If a user calls Promise.reject(null) in a somewhere, it bubbles to here and causes a further error. This should fix it

I'm not sure how to test this - I would think that it's in integration-test, but adding a test that looks a lot like this didn't seem to trip like i thought it would. Any ideas would be appreciated!

@xavdid xavdid requested a review from eliangcs October 5, 2018 01:57
Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Pulled and porked around a bit. I think we can't test it because here we're stopping the error from raising in the integration test runner. I tried testing on production but can't seem to do so. Did you test it on production? If so, what does your example app look like?

@xavdid
Copy link
Contributor Author

xavdid commented Oct 10, 2018

Yeah! I'm using CLI app 2174. The one currently pushed has a modified core.

Here's the trigger that shows the error:

perform: async (z, bundle) => {
    await Promise.reject(null)
    return [{ a: 1 }]
}

To see it in action, we want to look at the console_logs in GL:

So yeah. not sure we can unit test it, but it's fixed!

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Cool! I think we can take a pass on the unit test then!

@xavdid xavdid merged commit 0506544 into master Oct 10, 2018
@xavdid xavdid deleted the err-handling branch October 10, 2018 16:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants