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

'throws' doesn't work with async functions #33

Open
vitoreiji opened this issue Apr 23, 2021 · 4 comments
Open

'throws' doesn't work with async functions #33

vitoreiji opened this issue Apr 23, 2021 · 4 comments
Labels
enhancement New feature or request

Comments

@vitoreiji
Copy link

ospec version: 4.1.1
node version: 14.15.4

throws does not seem to catch exceptions from async functions.

Code

o("synchronous", function() {
    o(() => {throw new Error()}).throws(Error)
})
o("asynchronous", function() {
    o(async () => {throw new Error()}).throws(Error)
})

Expected behavior

––––––
All 2 assertions passed (old style total: 2) 

Current behavior

asynchronous:
[AsyncFunction (anonymous)]
  should throw a
[Function: Error] { stackTraceLimit: 10 }
    at /home/vis/dev/repositories/2020-04-pqmail-prototype/report.js:6:40
    
––––––
1 out of 2 assertions failed (old style total: 2)  
@dead-claudia
Copy link
Member

dead-claudia commented Apr 23, 2021

ospec will see a promise rejection, not a thrown exception, and you can't implicitly await within functions. (If you could, this wouldn't even be needed - just do the await within the inner function if you need to await a promise.)

I'll consider this a feature request/gotcha, not a bug.

@dead-claudia dead-claudia added the enhancement New feature or request label Apr 23, 2021
@vitoreiji
Copy link
Author

Then let me add some motivation to this feature request :)

Currently, the best way I know to assert that a given error is thrown is like this:

let error
try{
   await myFunction()
}catch(e){
    error = e
}
o(error.constructor).equals(MyError)

which looks really similar to the code that motivated the PR for throws/notThrows in the first place, except that it has an await.

Of course, if we don't want to use async/await, then the code would look more like this

let error
myFunction().catch(e => {
   error = e
}).finally(() => {
   o(error.constructor).equals(MyError)
   done()
})

I'd argue that this looks even more convoluted. Compare to

o(myFunction).throws(MyError)
// or even
o(async () => myFunction(foo)).throws(MyError)

These look much more readable to me.

Basically, the absence of this feature forces async/await users to revert back to the old style, which MithrilJS/mithril.js#2255 has established is not great.

@pygy
Copy link
Member

pygy commented Apr 8, 2022

Hello @vitoreiji this may prove useful.

asyncNotThrows = function(x) {
  return new Promise((f)=>{
    x()
    .then(
      v=>o().equals(), 
      e => o().satisfies(()=>({
      pass: false,
      message: 
`${x.toString()} shouldn't have thrown
      
${o.cleanStackTrace(e)}
`
    })))
    .finally(f)
  })
}

o("My async test", async function(){
  await asyncNotThrows(async ()=>{throw new Error("Caught by ospec !")})
})

@pygy
Copy link
Member

pygy commented May 20, 2022

The .satisfies() hook could be extended to return a promise, optionally.

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

No branches or pull requests

3 participants