raises-exception tag does not work in ipykernel 5.5.0

[frenzymadness]

Tests of nbclient fail with the latest ipykernel. I'm not sure why yet but it seems that the latest ipykernel does not respect raises-exception cell tag and stops executing. I'm using this notebook from nbclient tests to reproduce the issue.
The first cell has the mentioned tag so the raised exception should not stop the execution and the second cell should be executed as well. This is true with ipykernel 5.4.3, where the notebook after the execution looks like this:

{
 "cells": [
  {
   "cell_type": "code",
   "execution_count": 1,
   "metadata": {
    "execution": {
     "iopub.execute_input": "2021-02-24T09:28:28.153788Z",
     "iopub.status.busy": "2021-02-24T09:28:28.152885Z",
     "iopub.status.idle": "2021-02-24T09:28:28.200714Z",
     "shell.execute_reply": "2021-02-24T09:28:28.200111Z"
    },
    "tags": [
     "raises-exception"
    ]
   },
   "outputs": [
    {
     "ename": "Exception",
     "evalue": "message",
     "output_type": "error",
     "traceback": [
      "\u001b[0;31m---------------------------------------------------------------------------\u001b[0m",
      "\u001b[0;31mException\u001b[0m                                 Traceback (most recent call last)",
      "\u001b[0;32m<ipython-input-1-644b5753a261>\u001b[0m in \u001b[0;36m<module>\u001b[0;34m\u001b[0m\n\u001b[1;32m      1\u001b[0m \u001b[0;31m# üñîçø∂é\u001b[0m\u001b[0;34m\u001b[0m\u001b[0;34m\u001b[0m\u001b[0;34m\u001b[0m\u001b[0m\n\u001b[0;32m----> 2\u001b[0;31m \u001b[0;32mraise\u001b[0m \u001b[0mException\u001b[0m\u001b[0;34m(\u001b[0m\u001b[0;34m\"message\"\u001b[0m\u001b[0;34m)\u001b[0m\u001b[0;34m\u001b[0m\u001b[0;34m\u001b[0m\u001b[0m\n\u001b[0m",
      "\u001b[0;31mException\u001b[0m: message"
     ]
    }
   ],
   "source": [
    "# üñîçø∂é\n",
    "raise Exception(\"message\")"
   ]
  },
  {
   "cell_type": "code",
   "execution_count": 2,
   "metadata": {
    "execution": {
     "iopub.execute_input": "2021-02-24T09:28:28.308682Z",
     "iopub.status.busy": "2021-02-24T09:28:28.307285Z",
     "iopub.status.idle": "2021-02-24T09:28:28.313628Z",
     "shell.execute_reply": "2021-02-24T09:28:28.312340Z"
    }
   },
   "outputs": [
    {
     "name": "stdout",
     "output_type": "stream",
     "text": [
      "ok\n"
     ]
    }
   ],
   "source": [
    "print('ok')"
   ]
  }
 ],
 "metadata": {
  "language_info": {
   "codemirror_mode": {
    "name": "ipython",
    "version": 3
   },
   "file_extension": ".py",
   "mimetype": "text/x-python",
   "name": "python",
   "nbconvert_exporter": "python",
   "pygments_lexer": "ipython3",
   "version": "3.9.2"
  }
 },
 "nbformat": 4,
 "nbformat_minor": 1
}

but with the latest version, the second cell is not executed:

{
 "cells": [
  {
   "cell_type": "code",
   "execution_count": 1,
   "metadata": {
    "execution": {
     "iopub.execute_input": "2021-02-24T09:44:49.786558Z",
     "iopub.status.busy": "2021-02-24T09:44:49.786002Z",
     "iopub.status.idle": "2021-02-24T09:44:49.831791Z",
     "shell.execute_reply": "2021-02-24T09:44:49.831305Z"
    },
    "tags": [
     "raises-exception"
    ]
   },
   "outputs": [
    {
     "ename": "Exception",
     "evalue": "message",
     "output_type": "error",
     "traceback": [
      "\u001b[0;31m---------------------------------------------------------------------------\u001b[0m",
      "\u001b[0;31mException\u001b[0m                                 Traceback (most recent call last)",
      "\u001b[0;32m<ipython-input-1-644b5753a261>\u001b[0m in \u001b[0;36m<module>\u001b[0;34m\u001b[0m\n\u001b[1;32m      1\u001b[0m \u001b[0;31m# üñîçø∂é\u001b[0m\u001b[0;34m\u001b[0m\u001b[0;34m\u001b[0m\u001b[0;34m\u001b[0m\u001b[0m\n\u001b[0;32m----> 2\u001b[0;31m \u001b[0;32mraise\u001b[0m \u001b[0mException\u001b[0m\u001b[0;34m(\u001b[0m\u001b[0;34m\"message\"\u001b[0m\u001b[0;34m)\u001b[0m\u001b[0;34m\u001b[0m\u001b[0;34m\u001b[0m\u001b[0m\n\u001b[0m",
      "\u001b[0;31mException\u001b[0m: message"
     ]
    }
   ],
   "source": [
    "# üñîçø∂é\n",
    "raise Exception(\"message\")"
   ]
  },
  {
   "cell_type": "code",
   "execution_count": 2,
   "metadata": {
    "execution": {
     "iopub.status.busy": "2021-02-24T09:44:49.833859Z",
     "iopub.status.idle": "2021-02-24T09:44:49.834236Z",
     "shell.execute_reply": "2021-02-24T09:44:49.833506Z"
    }
   },
   "outputs": [],
   "source": [
    "print('ok')"
   ]
  }
 ],
 "metadata": {
  "language_info": {
   "codemirror_mode": {
    "name": "ipython",
    "version": 3
   },
   "file_extension": ".py",
   "mimetype": "text/x-python",
   "name": "python",
   "nbconvert_exporter": "python",
   "pygments_lexer": "ipython3",
   "version": "3.9.2"
  }
 },
 "nbformat": 4,
 "nbformat_minor": 1
}

I'm not sure yet where the root cause is laying. Any help appreciated.

[frenzymadness]

But I'm not able to reproduce the same issue in jupyter notebook so the issue might be in nbclient. I'm gonna report it there as well.

[davidbrochart]

From what I can see, the bug was introduced by these commits:
3c652de#diff-10179407bb8db6eccacc6da9ca62c2bbbb03bceec4db91e8a021d473e26a5d6c
9655cfd#diff-10179407bb8db6eccacc6da9ca62c2bbbb03bceec4db91e8a021d473e26a5d6c
45afaab#diff-10179407bb8db6eccacc6da9ca62c2bbbb03bceec4db91e8a021d473e26a5d6c
They are not documented in the changelog. Maybe @glentakahashi knows more what the issue could be?

[davidbrochart]

More precisely, reverting this commit fixes the issue:
45afaab#diff-10179407bb8db6eccacc6da9ca62c2bbbb03bceec4db91e8a021d473e26a5d6c

[glentakahashi]

So the changes I added were intended to fix a bug with the above behavior. You can see #572 and #539 for more details.

By default, ipykernel is supposed to abort other cell runs when on cell encounters an error. You can see this normally by calling "run all" on a notebook where a cell in the middle has an error. Execution will run until the cell with the error and then stop aftewards.

ipykernel had a bug before where its builtin default of aborting other cells after one cell encounters an error didn't work properly with timeouts, so sometimes you would end up in scenarios where cells after an error would still run.

I think for that test in nbclient, the correct behavior is that cell #2 is not executed, and that previously it only executed because of the above bug.

EDIT: sorry i totally missed your main point. I'm not aware of what the "raises-exception" tag is. I can't see that mentioned anywhere in the jupyter or ipykernel codebase?

[glentakahashi]

Ah sorry, I did some more research and now understand what you're talking about (this is what i get for replying too early). I can't find any docs on the "raises-exception" tag, but I did find the code and I think? I know how its expected to work. If so, I think the bug is in nbclient on this line: https://github.com/jupyter/nbclient/blob/df1cecb9daf3b64c6714d92d7f3ab26f63a04c91/nbclient/client.py#L814 and the fix should be to conditionally set that flag on something like not self.allow_errors and "raises-exception" not in cell.metadata.get("tags", []).

I don't think I understand the expected behavior entirely though, is execution supposed to still continue even if the cell doesn't match one of the allowed exception names? Ipykernel doesn't have support for whitelisting specific types of exceptions, just the wholesale stop_on_error flag, so it'd be hard to properly implement the complex conditional logic that nbclient does for which types of exceptions are/aren't allowed to keep execution

[davidbrochart]

I agree it's not very intuitive, but the logic is that if a cell is tagged with raises-exception, that will allow errors in the cell execution (i.e. NOT raise an exception 😄).
You are right, the bug is on this exact line in nbclient, where we forgot to include more conditions in the stop_on_error parameter. In this particular case, it should not stop when the cell is tagged with raises-exception. So I guess the bug you fixed in ipykernel revealed a bug in nbclient.
Anyways, thank you very much for you help! I will open a PR in nbclient and close this issue.

[davidbrochart]

nbclient v0.5.3 has been released, which should fix this issue.

[frenzymadness]

I can confirm that all tests are passing now. Thank you for the quick action!