Repository: opentrons/opentrons
Issue: (v3) SessionManager.create emits a Session notification even if creation fails

## overview

Currently, the API/app does not handle a rejected session creation due to the way the API sends out session notifications and the assumptions the app makes when receiving session notifications.

This issue is for:

- [X] Bug, feature request, or other? **Bug**
- [X] API, App, both? **API or App, depending on how we fix it**
- [X] Version? **v3**
- [X] OS and OS version? **N/A**

## expected or desired behavior

If I call `session_manager.create(...)` from the client and get an error, the app expects not to receive any subsequent session state change notifications, because as far as the app is concerned, no session exists.

We need to do one of two things:

* (API, my preference) Do not emit session notifications for sessions that we're not successfully created
* (App) Refactor app session handling to account for session notifications for sessions that failed during creation

## actual behavior

If `create` fails (for example, if the protocol passed to create had an error), the client receives the method call rejection (which is good) but then subsequently receives a session state change notification for the session that was not successfully created. The session notification it receives has errors in its `errors` array.

At the moment, the client is written in a way that assumes any session notifications are for active, real sessions, and that `errors` in those session notifications are for problems during a real protocol run. Because of this, receiving a notification for a "bad" session (as opposed to a "good" session that errors out during a run) confuses the client and throws the state out of whack.

## steps to reproduce problem

The best way to observe this problem is to watch the Redux log when you're doing this:

1. Connect the app to the API
2. Upload a protocol with syntax errors
3. In the log, observe:
    1. a `SESSION_RESPONSE` action comes in with `error` set to true (the create call rejection)
    2. Then, another `SESSION_RESPONSE` action come in with `error: false` (the session notification)


Changed files:
api/opentrons/api/session.py, api/tests/opentrons/conftest.py, api/tests/opentrons/api/test_session.py

diff --git a/api/opentrons/api/session.py b/api/opentrons/api/session.py
index f708f7562e..448162a3e5 100644
--- a/api/opentrons/api/session.py
+++ b/api/opentrons/api/session.py
@@ -135,19 +135,16 @@ class Session(object):
     def refresh(self):
         self._reset()
 
-        try:
-            parsed = ast.parse(self.protocol_text)
-            self._protocol = compile(parsed, filename=self.name, mode='exec')
-            commands = self._simulate()
-            self.commands = tree.from_list(commands)
-        finally:
-            if self.errors:
-                raise Exception(*self.errors)
+        parsed = ast.parse(self.protocol_text)
+        self._protocol = compile(parsed, filename=self.name, mode='exec')
+        commands = self._simulate()
+        self.commands = tree.from_list(commands)
+
+        self.containers = self.get_containers()
+        self.instruments = self.get_instruments()
 
-            self.containers = self.get_containers()
-            self.instruments = self.get_instruments()
+        self.set_state('loaded')
 
-            self.set_state('loaded')
         return self
 
     def stop(self):
diff --git a/api/tests/opentrons/api/test_session.py b/api/tests/opentrons/api/test_session.py
index 807cbfbaaf..eeb5cd2744 100644
--- a/api/tests/opentrons/api/test_session.py
+++ b/api/tests/opentrons/api/test_session.py
@@ -5,6 +5,10 @@ from datetime import datetime
 from opentrons.broker import publish
 from opentrons.api import Session
 from opentrons.api.session import _accumulate, _get_labware, _dedupe
+from conftest import state
+from functools import partial
+
+state = partial(state, 'session')
 
 
 @pytest.fixture
@@ -69,12 +73,7 @@ async def test_load_protocol_with_error(session_manager):
         assert session is None
 
     args, = e.value.args
-    timestamp = args['timestamp']
-    exception = args['error']
-
-    assert datetime.strptime(timestamp, '%Y-%m-%dT%H:%M:%S.%f')
-    assert type(exception) == NameError
-    assert str(exception) == "name 'blah' is not defined"
+    assert args == "name 'blah' is not defined"
 
 
 @pytest.mark.parametrize('protocol_file', ['testosaur.py'])
@@ -263,3 +262,23 @@ async def test_drop_tip_with_trash(session_manager, protocol, protocol_file):
     assert 'trash-box' in [c.name for c in session.get_containers()]
     containers = sum([i.containers for i in session.get_instruments()], [])
     assert 'trash-box' in [c.name for c in containers]
+
+
+async def test_session_create_error(main_router):
+    with pytest.raises(SyntaxError):
+        main_router.session_manager.create(
+            name='<blank>',
+            text='syntax error ;(')
+
+    with pytest.raises(TimeoutError):
+        # No state change is expected
+        await main_router.wait_until(state('loaded'))
+
+    with pytest.raises(ZeroDivisionError):
+        main_router.session_manager.create(
+            name='<blank>',
+            text='1/0')
+
+    with pytest.raises(TimeoutError):
+        # No state change is expected
+        await main_router.wait_until(state('loaded'))
diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py
index b20a881a36..515ddbe279 100644
--- a/api/tests/opentrons/conftest.py
+++ b/api/tests/opentrons/conftest.py
@@ -196,6 +196,7 @@ async def wait_until(matcher, notifications, timeout=1, loop=None):
         done, pending = await asyncio.wait([coro], timeout=timeout)
 
         if pending:
+            [task.cancel() for task in pending]
             raise TimeoutError('Notifications: {0}'.format(result))
 
         result += [done.pop().result()]