1392070 - Stop using StopIteration in Sqlite.jsm

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, enhancement)

Description

[Masatoshi Kimura [:emk]]

Sqlite.jsm is abusing StopIteration to indicate user cancelling in onRow callback.
It does not even follow the legacy iteration protocol.

We should use something different to drop StopIteration. Fortunately, we don't have to concern addon-compat anymore.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Bug 1392070 - Stop using the StopIteration object in Sqlite.jsm.

Review commit: https://reviewboard.mozilla.org/r/170498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170498/

Comment 2

[Masatoshi Kimura [:emk]]

This patch is using a string instead of the global singleton. I'd appreciate better suggestions.

Comment 3

[Marco Bonardo [:mak]]

Maybe we could change the onRow signature from onRow(row) to onRow(row, canceler), so that the onRow consumer can invoke canceler(). Internally it can just do the same as the current StopIteration exception.

Comment 4

[Florian Quèze [:florian]]

Comment on attachment 8899246 [details]
Bug 1392070 - Stop using the StopIteration object in Sqlite.jsm.

Removing the review flag as Marco offered a suggestion.

Comment 5

[Masatoshi Kimura [:emk]]

Comment on attachment 8899246 [details]
Bug 1392070 - Stop using the StopIteration object in Sqlite.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170498/diff/1-2/

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 8899246 [details]
Bug 1392070 - Stop using the StopIteration object in Sqlite.jsm.

https://reviewboard.mozilla.org/r/170498/#review176234

::: toolkit/modules/Sqlite.jsm:1286
(Diff revision 2)
>     * resources.
>     *
>     * If a `StopIteration` is thrown during execution of an `onRow` handler,
>     * the execution of the statement is immediately cancelled. Subsequent
>     * rows will not be processed and no more `onRow` invocations will be made.
>     * The promise is resolved immediately.

This comment must be updated

::: toolkit/modules/Sqlite.jsm:1290
(Diff revision 2)
>     * rows will not be processed and no more `onRow` invocations will be made.
>     * The promise is resolved immediately.
>     *
>     * If a non-`StopIteration` exception is thrown by the `onRow` handler, the
>     * exception is logged and processing of subsequent rows occurs as if nothing
>     * happened. The promise is still resolved (not rejected).

as well as this one

Comment 7

[Masatoshi Kimura [:emk]]

Comment on attachment 8899246 [details]
Bug 1392070 - Stop using the StopIteration object in Sqlite.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170498/diff/2-3/

Comment 8

[Masatoshi Kimura [:emk]]

Comment on attachment 8899246 [details]
Bug 1392070 - Stop using the StopIteration object in Sqlite.jsm.

https://reviewboard.mozilla.org/r/170498/#review176234

> This comment must be updated

Comments updated.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9ae810f6a04
Stop using the StopIteration object in Sqlite.jsm. r=mak

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b9ae810f6a04