Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Daniel Hahler):

 Ok, fair enough - it is only fixing a regression on Python 3 then (for
 me), that was actually done as a workaround.

 I suggest reading the original tickets description maybe then: my
 intention was to not mask / "extend" the actual exception with more
 information (the (IMHO incorrectly chained) error during cleanup, that is
 expected to fail).  It is similar to a `file.close()` when it is expected
 that it wasn't created or its handler being invalid already.

 Replying to
 
https://github.com/django/django/commit/6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf#r67292630:

 > The cleanup part of the chain adds noise to the actual source of the
 error, so maybe it can be improved. But that’s an improvement over the
 existing situation, not a regression.

 Ok, I see.  That's what I've meant to do: basically restoring the behavior
 with Python 3 (when the workaround for Python 2 was in place), while being
 a bit smarter to only ignore/swallow expected errors.
 I was not aware that the intention with the Python 2 workaround was to
 chain ("add useless noise") always with Python 3.

 The question here really is also: if you would know that the connection is
 invalid, and that `close()` would throw: would you call it in the first
 place?

 As for a new PR I'd rather update the existing one, but cannot re-open it
 as mentioned there.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.b98be212b2d500ea0a5e8dbc14782482%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Carlton Gibson):

 Hi Daniel.

 It wasn't really that 6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf ignored all
 exceptions. Rather it stopped the source exception being masked, with the
 explicit intention that the new chained behaviour would be preferred when
 it was available:

 {{{
 # Ignore clean up errors and raise the original error instead.
 # Python 2 doesn't chain exceptions. Remove this error
 # silencing when dropping Python 2 compatibility.
 }}}

 Given that it's clearly not a regression in
 d170c63351944fd91b2206d10f89e7ff75b53b76 — that was the intended change
 all along.

 You're then asking for a change in behaviour to **remove** information
 from the traceback. Maybe that's OK if there's a good reason, but
 otherwise the presumption would be wontfix.

 I'm not sure about the patch itself: it would be easier to consider in a
 fresh PR, with a regression test.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.9793b2e73455a0a4ee494d6db7e979d8%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Daniel Hahler):

 https://code.djangoproject.com/ticket/16614#comment:31 is about "Refs
 #16614 -- Prevented database errors from being masked by cursor close.".

 The fix there ignored all exceptions
 
(https://github.com/django/django/commit/6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf).
 The fix I've done for the regression is a bit smarter to still raise in
 case of unexpected errors.
 (This could also be removed in case you want a more concise patch, but
 even when it is unlikely that it would swallow something unwanted, I've
 meant to put attention to detail there - just in line with the difference
 that we've previously discussed here)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.110f13a2335a47b2a40e35cc067ee566%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Carlton Gibson):

 > The main thing … is not raising errors from cursor.close() in the first
 place … e.g. when it gets used for display in the short test summary etc.

 So the request is suppressing the chained nested exception — you don't
 want the newer Python 3 behaviour in this case — which would be a new
 feature, rather than a fixing a regression from #16614.

 And the reason you want this if because your test runner (presumably
 pytest) formats its output in a given way? (Is that it?)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.e1408392b3c6fc0427b26d973159b7e9%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Daniel Hahler):

 The main thing this is meant to fix is not raising errors from
 `cursor.close()` in the first place.
 The difference between "caused by" and "then triggered also" is only
 relevant for when it should not get swallowed (based on if it is expected
 because there was an error already).

 What you currently get during a test run with a missing migration:
 {{{
 Traceback (most recent call last):
   File "…/Vcs/django/django/db/backends/utils.py", line 89, in _execute
 return self.cursor.execute(sql, params)
 psycopg2.errors.UndefinedColumn: column foo.bar does not exist
 LINE 1: ...
  ^


 The above exception was the direct cause of the following exception:

 Traceback (most recent call last):
   File "…/Vcs/django/django/db/models/sql/compiler.py", line 1361, in
 execute_sql
 cursor.execute(sql, params)
 …
   File "…/Vcs/django/django/db/backends/utils.py", line 89, in _execute
 return self.cursor.execute(sql, params)
 django.db.utils.ProgrammingError: column foo.bar does not exist
 LINE 1: ...
  ^


 During handling of the above exception, another exception occurred:

 Traceback (most recent call last):
   File "…/project/.venv/lib/python3.10/site-
 packages/pytest_django/fixtures.py", line 122, in django_db_setup
 db_cfg = setup_databases(
 …
   File "…/Vcs/django/django/db/models/sql/compiler.py", line 1364, in
 execute_sql
 cursor.close()
 psycopg2.errors.InvalidCursorName: cursor
 "_django_curs_140147764090688_sync_10" does not exist
 }}}

 `psycopg2.errors.InvalidCursorName` is expected to happen here, and
 obfuscates/hides the real error (e.g. when it gets used for display in the
 short test summary etc).

 The (current) patch changes it to:
 {{{
 Traceback (most recent call last):
   File "…/Vcs/django/django/db/backends/utils.py", line 89, in _execute
 return self.cursor.execute(sql, params)
 psycopg2.errors.UndefinedColumn: column foo.bar does not exist
 LINE 1: ...
  ^


 The above exception was the direct cause of the following exception:

 Traceback (most recent call last):
   File "…/project/.venv/lib/python3.10/site-
 packages/pytest_django/fixtures.py", line 122, in django_db_setup
 db_cfg = setup_databases(
 …
   File "…/Vcs/django/django/db/backends/utils.py", line 89, in _execute
 return self.cursor.execute(sql, params)
 django.db.utils.ProgrammingError: column foo.bar does not exist
 LINE 1: ...
  ^
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.61ad0bde2429328a2a3199fc7082d9fa%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * status:  new => closed
 * resolution:   => needsinfo


Comment:

 Hi Daniel.

 I'm really sorry but I'm failing to see the point you're trying to make
 here... There's clearly something I'm missing because you keep trying to
 get it across to me, but I just don't get it. Can I **please** ask you to
 take a step back and explain it to me from the top clearly so that I can
 see the issue?

 It looks to me like the code we have here when there's no exception in
 `cursor.close()` is equivalent to this:


 {{{
 def will_raise(msg):
 raise Exception(msg)


 try:
 will_raise("Outer")
 except Exception:
 raise
 }}}

 When I run that, I get this output:


 {{{
 % python3 exceptions.py
 Traceback (most recent call last):
   File "/Users/carlton/Desktop/exceptions.py", line 6, in 
 will_raise("Outer")
   File "/Users/carlton/Desktop/exceptions.py", line 2, in will_raise
 raise Exception(msg)
 Exception: Outer
 }}}

 When there **is** an exception in `cursor.close()` it looks like this:

 {{{
 def will_raise(msg):
 raise Exception(msg)


 try:
 will_raise("Outer")
 except Exception:
 will_raise("Inner")
 raise
 }}}


 Which gives this output:


 {{{
 % python3 exceptions.py
 Traceback (most recent call last):
   File "/Users/carlton/Desktop/exceptions.py", line 6, in 
 will_raise("Outer")
   File "/Users/carlton/Desktop/exceptions.py", line 2, in will_raise
 raise Exception(msg)
 Exception: Outer

 During handling of the above exception, another exception occurred:

 Traceback (most recent call last):
   File "/Users/carlton/Desktop/exceptions.py", line 8, in 
 will_raise("Inner")
   File "/Users/carlton/Desktop/exceptions.py", line 2, in will_raise
 raise Exception(msg)
 Exception: Inner
 }}}


 Your suggestion seems to be to make it equivalent to:


 {{{
 def will_raise(msg):
 raise Exception(msg)


 try:
 will_raise("Outer")
 except Exception as o:
 try:
 will_raise("Inner")
 except Exception as i:
 raise i from o
 raise
 }}}

 ... which has the output:


 {{{
 % python3 exceptions.py
 Traceback (most recent call last):
   File "/Users/carlton/Desktop/exceptions.py", line 19, in 
 will_raise("Outer")
   File "/Users/carlton/Desktop/exceptions.py", line 15, in will_raise
 raise Exception(msg)
 Exception: Outer

 The above exception was the direct cause of the following exception:

 Traceback (most recent call last):
   File "/Users/carlton/Desktop/exceptions.py", line 24, in 
 raise i from o
   File "/Users/carlton/Desktop/exceptions.py", line 22, in 
 will_raise("Inner")
   File "/Users/carlton/Desktop/exceptions.py", line 15, in will_raise
 raise Exception(msg)
 Exception: Inner
 }}}

 Which is exactly the same bar the "direct cause" wording change, that was
 agreed on the mailing list discussion to be not worth the verbosity of the
 code change.

 So what's it fixing? (Again: Sorry to not be seeing this, but I'm just not
 getting the point you want to make.)

 > It is meant to fix
 https://code.djangoproject.com/ticket/16614#comment:31 again, which
 regressed.

 But it didn't regress. Both `Outer` and `Inner` are given in the
 traceback. `Outer` is given first, plus the additional info that `Inner`
 occurred whilst we were handling that.
 This behaviour in Python 3 is what justifies the change in
 
https://github.com/django/django/commit/d170c63351944fd91b2206d10f89e7ff75b53b76
 #diff-
 f58de2deaccecd2d53199c5ca29e3e1050ec2adb80fb057cdfc0b4e6accdf14fR878-R879
 — the original error **is** raised.

 If you can put together an actual example of the problem you're hitting
 coming up, that might make it easier. It's very hard to see when it's just
 a couple of lines changed with no test-case or reproduce. (I appreciate
 cursor errors can be hard to test...)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the 

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Daniel Hahler):

 * status:  closed => new
 * resolution:  wontfix =>


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.19479d9a9126e8132657a9074d3ce513%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Daniel Hahler):

 The main issue here is not about how to raise in case it should be raised,
 but to not raise in the first place, if an error is expected.
 It is meant to fix https://code.djangoproject.com/ticket/16614#comment:31
 again, which regressed.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.389a4cd0d683b53ba76d3a476d878909%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Carlton Gibson):

 Hi Daniel. Thanks for the reply.

 It's OK to not read the thread :)

 > ...not about context or cause, but only using raise from properly
 (according to...

 It's the force of ''properly'' here that's in doubt.

 On investigation, the difference between the two syntaxes amounted to no
 more (not much more) that the text used between the tracebacks — which are
 chained either way, either ''implicitly'' or ''explicitly'', unless `from
 None` is used. That difference in text, ''"During handling of the above
 exception, another exception occurred"'' vs ''"The above exception was the
 direct cause of the following exception"'', wasn't felt significant — not
 compared to the more verbose syntax, and in the PR you have here, much
 more verbose block in order to decide when to use it.

 Thus, with the exceptions noted above, it was decided not to use the
 `from` syntax. (We didn't agree with the blog post that not using it was
 ''improper''.)

 So, if there's a concrete example, of ''"Look, you get this benefit"'',
 then I'm really happy to look at that, and learn a trick, but pending
 that, we have to go with the previous consensus. I hope that makes sense.
 Thanks!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.ce34f5782945858172c14d0383b7c69a%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Daniel Hahler):

 Sorry for not having read the mailing list thread yet, but only your
 summary of it above.
 I think the main issue with context vs. cause is an important detail.  I
 can see that it might not worth changing it all over the code base, but it
 should be considered when fixing code.

 The main issue I see here is that there will be an exception almost always
 (at least with named cursors in PostgreSQL AFAICS), and therefore it
 should not even be chained in this case: it really just hides the actual
 error, just because it is trying to ensure a (never properly opened/used)
 cursor is closed always.

 I might follow up on the mailing list, but do not really feel like
 subscribing there also etc, especially since the issue here is not about
 context or cause, but only using `raise from` properly (according to
 https://blog.ram.rachum.com/post/621791438475296768/improving-python-
 exception-chaining-with) when not swallowing/hiding the expected
 exception.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.74c47913aeea05683a575dc75e8abf8c%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * status:  new => closed
 * resolution:   => wontfix


Comment:

 There was an extended discussion of the merits of the `raise...from`
 syntax around #31177 ([https://groups.google.com/g/django-
 developers/c/ibEOt3A9c2M/m/sOL7YV5LFwAJ Mailing list thread]).

 The summary of that was that, in general, given that exceptions are
 automatically chained, the different message **between** the chained
 exceptions was not worth the noise.

 The cited exceptions were using `raise...from` to remove noise from an
 long-chain of exceptions, when you wished to pull-out the **real cause**
 that may perhaps lie a frame frames deep, and using `from None` to
 suppress chaining, when the underlying exception is not informative.

 I'm going to close as wontfix on the basis of that discussion. If you
 think this case is different you can message the DevelopersMailingList.
 (If so, please give a concrete example of how the change is supposed to
 help — I couldn't really see if from the description here — it seems like
 a lot of effort to change the message between the tracebacks, but maybe
 you're using the `__cause__` attribute in a way that merits it…? 樂)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.783d21bcfa6b3dfb30389002ffd154d5%40djangoproject.com.

Re: [Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
 Reporter:  Daniel Hahler|Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Daniel Hahler):

 * has_patch:  0 => 1


Old description:

> `cursor.close()` is explicitly after Exceptions with `cursor.execute()`,
> and has a comment already that it might fail:
> https://github.com/django/django/blob/322a1a037d4d2f18744c5d1a1efc2e84d4c5e94b/django/db/models/sql/compiler.py#L1210-L1215
>
> {{{
> try:
> cursor.execute(sql, params)
> except Exception:
> # Might fail for server-side cursors (e.g. connection closed)
> cursor.close()
> }}}
>
> The code there was changed to ignore any exception in
> https://github.com/django/django/commit/6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf
> (Refs #16614), but then changed to the current code in
> https://github.com/django/django/commit/d170c63351944fd91b2206d10f89e7ff75b53b76
> (https://github.com/django/django/commit/d170c63351944fd91b2206d10f89e7ff75b53b76
> #diff-f58de2deaccecd2d53199c5ca29e3e1050ec2adb80fb057cdfc0b4e6accdf14f).
>
> The current approach however will create a chained exception always, and
> especially will add it as context, and not as cause (see
> https://blog.ram.rachum.com/post/621791438475296768/improving-python-
> exception-chaining-with).
>
> I suggest that exceptions in `cursor.close()` should get ignored as being
> expected in a lot / most cased, and it being done only to ensure the
> connection gets closed really, and should get `raised from` otherwise.
>
> The main motivation here is that it makes it hard/confusing to see the
> real error, e.g. when running tests or doing development, and the DB is
> out of sync.

New description:

 `cursor.close()` is explicitly after Exceptions with `cursor.execute()`,
 and has a comment already that it might fail:
 
https://github.com/django/django/blob/322a1a037d4d2f18744c5d1a1efc2e84d4c5e94b/django/db/models/sql/compiler.py#L1210-L1215

 {{{
 try:
 cursor.execute(sql, params)
 except Exception:
 # Might fail for server-side cursors (e.g. connection closed)
 cursor.close()
 }}}

 The code there was changed to ignore any exception in
 
https://github.com/django/django/commit/6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf
 (Refs #16614), but then changed to the current code in
 
https://github.com/django/django/commit/d170c63351944fd91b2206d10f89e7ff75b53b76
 
(https://github.com/django/django/commit/d170c63351944fd91b2206d10f89e7ff75b53b76
 #diff-f58de2deaccecd2d53199c5ca29e3e1050ec2adb80fb057cdfc0b4e6accdf14f).

 The current approach however will create a chained exception always, and
 especially will add it as context, and not as cause (see
 https://blog.ram.rachum.com/post/621791438475296768/improving-python-
 exception-chaining-with).

 I suggest that exceptions in `cursor.close()` should get ignored as being
 expected in a lot / most cased, and it being done only to ensure the
 connection gets closed really, and should get `raised from` otherwise.

 The main motivation here is that it makes it hard/confusing to see the
 real error, e.g. when running tests or doing development, and the DB is
 out of sync.

 PR: https://github.com/django/django/pull/15141

--

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.525d17d665d7f61a42792fa11f9538c6%40djangoproject.com.

[Django] #33331: Improve exception handling with `cursor.close()` after errors

[Django]

#1: Improve exception handling with `cursor.close()` after errors
-+-
   Reporter:  Daniel |  Owner:  nobody
  Hahler |
   Type: | Status:  new
  Cleanup/optimization   |
  Component:  Database   |Version:  3.2
  layer (models, ORM)|
   Severity:  Normal |   Keywords:
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+-
 `cursor.close()` is explicitly after Exceptions with `cursor.execute()`,
 and has a comment already that it might fail:
 
https://github.com/django/django/blob/322a1a037d4d2f18744c5d1a1efc2e84d4c5e94b/django/db/models/sql/compiler.py#L1210-L1215

 {{{
 try:
 cursor.execute(sql, params)
 except Exception:
 # Might fail for server-side cursors (e.g. connection closed)
 cursor.close()
 }}}

 The code there was changed to ignore any exception in
 
https://github.com/django/django/commit/6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf
 (Refs #16614), but then changed to the current code in
 
https://github.com/django/django/commit/d170c63351944fd91b2206d10f89e7ff75b53b76
 
(https://github.com/django/django/commit/d170c63351944fd91b2206d10f89e7ff75b53b76
 #diff-f58de2deaccecd2d53199c5ca29e3e1050ec2adb80fb057cdfc0b4e6accdf14f).

 The current approach however will create a chained exception always, and
 especially will add it as context, and not as cause (see
 https://blog.ram.rachum.com/post/621791438475296768/improving-python-
 exception-chaining-with).

 I suggest that exceptions in `cursor.close()` should get ignored as being
 expected in a lot / most cased, and it being done only to ensure the
 connection gets closed really, and should get `raised from` otherwise.

 The main motivation here is that it makes it hard/confusing to see the
 real error, e.g. when running tests or doing development, and the DB is
 out of sync.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/050.c64eb6b3d30db6fd71c3eba9d814940d%40djangoproject.com.