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

Improve error message for createInteractor #820

Closed
jenweber opened this issue Feb 4, 2021 · 4 comments · Fixed by #931
Closed

Improve error message for createInteractor #820

jenweber opened this issue Feb 4, 2021 · 4 comments · Fixed by #931
Assignees
Labels
🐛bug error handling issues related to better error handling and communication

Comments

@jenweber
Copy link
Collaborator

jenweber commented Feb 4, 2021

If you don't pass a string to createInteractor, you get the following message:

ERROR Cannot read property 'length' of undefined

It would be better for this error to be caught and have a message like:

"Please provide a description for your interactor, such as createInteractor('username input')

@cowboyd cowboyd added 🐛bug error handling issues related to better error handling and communication labels Feb 4, 2021
@minkimcello
Copy link
Contributor

convoluted mumbo jumbo
So I've looked into this and it seems we only need to worry about javascript tests because in typescript it'll tell you what's wrong before it tries to instantiate the interactors:

So with the introduction of the extend() API, we now have to cover two different scenarios (if we want to display the error message you suggested). But first I installed the sample app to investigate exactly what the current behaviour is.

I found that for both createInteractor() and extend(), it will not throw an error as long as the tests pass (they both funnel through the same constructor so it's not a surprise that they both behave the same way). Not passing in a name only becomes an issue when a test fails. In the bigtest runner, we get a ERROR h is undefined and for both jest and cypress, we get TypeError: Cannot read property 'length' of undefined.


My initial thought was to throw an error inside the constructor so that when an interactor is created (or extended), it'll display an error message to remind users to pass in a string. But, I don't think this would be the best approach because the stack trace doesn't tell us which interactor it is having issues with.

Here's what it looks like when we throw an error (nooo) inside the constructor:
Screen Shot 2021-05-14 at 6 48 29 AM

And here's what it looks like currently with the bigtest runner:
Screen Shot 2021-05-14 at 6 48 10 AM

And in jest:
Screen Shot 2021-05-14 at 7 02 22 AM

In a way, it's easier to track down which interactor is missing a name as it is currently. I could go into the bigtest runner and display a warning each time an interactor's name is "undefined" but then we'd have to find a solution for jest and cypress separately. Thoughts?

@minkimcello minkimcello self-assigned this May 14, 2021
@cowboyd
Copy link
Member

cowboyd commented May 18, 2021

@minkimcello @jenweber There are a couple of options:

  1. what if we just made a default anonymous interactor and it is not an error
  2. we defer the error from when you construct the interactor until you use the interactor
  3. we defer and make it a warning.

Hard to say which is best.

@minkimcello
Copy link
Contributor

@minkimcello @jenweber There are a couple of options:

1. what if we just made a default `anonymous interactor` and it is not an error

2. we defer the error from when you construct the interactor until you use the interactor

3. we defer and make it a warning.

Hard to say which is best.

Oh I spoke too soon earlier. I realized I can throw in the console.warn inside findMatches() to get the behaviour we're looking for.

I don't think we should have an anonymous interactor. That just creates one more thing we need to explain.

And I think we should throw a hard error to force people to use interactors properly. It also formats more nicely.

As a console warn:
Screen Shot 2021-05-18 at 7 40 24 AM

When we throw an error:
Screen Shot 2021-05-18 at 7 41 02 AM

@cowboyd
Copy link
Member

cowboyd commented May 19, 2021

Perfect!

So sounds like the options are:

  1. whiny anonymous interactor
  2. fail if no name

I kinda lean towards the (2) but don't feel strongly either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug error handling issues related to better error handling and communication
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants