Skip to content

fix: use errstr() instead of toSource() in nouveau handleIndexError for QuickJS compatibility#5951

Open
vamsi2246 wants to merge 2 commits intoapache:mainfrom
vamsi2246:fix/nouveau-tosource-quickjs-compat
Open

fix: use errstr() instead of toSource() in nouveau handleIndexError for QuickJS compatibility#5951
vamsi2246 wants to merge 2 commits intoapache:mainfrom
vamsi2246:fix/nouveau-tosource-quickjs-compat

Conversation

@vamsi2246
Copy link
Copy Markdown

Overview

handleIndexError() in nouveau.js calls err.toSource(), which is a SpiderMonkey-specific method not available in QuickJS. This causes the error handler itself to crash when a Nouveau index function throws a non-fatal error on QuickJS-based deployments.

This replaces err.toSource() with the existing errstr() helper (defined in util.js), which safely checks for toSource availability before calling it, falling back to toString(). This is consistent with the identical pattern used in dreyfus.js and views.js.

Testing recommendations

N/A - This is a trivial 1-line compatibility fix that aligns nouveau.js with the existing pattern in dreyfus.js.

vamsi and others added 2 commits March 29, 2026 00:53
Replace SpiderMonkey-specific err.toSource() with the cross-engine
errstr() helper in nouveau.js handleIndexError(). The errstr() function
(defined in util.js) safely checks for toSource availability before
calling it, falling back to toString(). This is consistent with the
identical pattern used in dreyfus.js handleIndexError().

Without this fix, any non-fatal error thrown by a Nouveau index function
on QuickJS causes the error handler itself to crash with a TypeError,
preventing graceful error logging and document skipping.
@nickva
Copy link
Copy Markdown
Contributor

nickva commented Mar 30, 2026

Thanks for your contribution, @vamsi2246

Please fill out the PR template.

Coincidentally, there is a parallel discussion in another PR #5943 (review) about how errstr can be improved. It's great to see a lot of activity and interest in improving js dispatching in CouchDB!

Wonder if we could cover this case by a test. It should be enough just to copy and paste a test from nouveau Elixir test and make it throw, like so:

  test "index function throws", context do
    db_name = context[:db_name]
    create_search_docs(db_name)
    ddoc =  %{
      nouveau: %{
        bar: %{
          default_analyzer: "standard",
          index: """
            function (doc) {
              throw(new Error("some error"));
              index("string", "foo", doc.foo, {store: true});
            }
          """
        }
      }
    }
    create_ddoc(db_name, ddoc)
    url = "/#{db_name}/_design/foo/_nouveau/bar"
    resp = Couch.post(url, body: %{q: "foo:bar", include_docs: true})
    assert_status_code(resp, 200)
    assert resp.body["hits"] == []
  end

Currently with QuickJS it should blow up with a 500 response and some ugly looking stacktrace. With this fix it should 1) return a 200 response 2) return no hits, 3) hopefully log something like "OS Process ... function raised ...". I don't think we can check for that in elixir but we hopefully will log locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants