Update wrap_jsonp to handle lists

[jimchamp]

Addresses regression identified here

starlette.responses.Response.render is throwing an AttributeError when data is a list. These changes ensure that lists are also serialized.

Technical

Testing

Screenshot

Stakeholders

@RayBB
@cdrini

[Copilot]

Pull request overview

Updates the FastAPI wrap_jsonp helper to correctly JSON-serialize list responses, preventing starlette.responses.Response.render errors when endpoints return [].

Changes:

• Expand wrap_jsonp input type to accept list in addition to dict/str.
• Serialize dict/list inputs via json.dumps while preserving pre-serialized JSON strings.
• Add an explicit type guard for unexpected data types.

Comments suppressed due to low confidence (1)

openlibrary/fastapi/models.py:77

• wrap_jsonp raises ValueError when the callback query param is invalid. Since this helper is used directly in FastAPI route handlers (e.g., openlibrary/fastapi/books.py) and there is no global exception handler for ValueError in the ASGI app, this will surface as a 500 error for a client-supplied bad callback. Prefer raising HTTPException(status_code=400, ...) so invalid user input returns a 4xx response (and update the existing unit test that currently expects ValueError).

    if callback := request.query_params.get("callback"):
        if not JS_CALLBACK_RE.match(callback):
            raise ValueError("Invalid callback parameter: must be a valid JavaScript identifier (only letters, numbers, underscore, $, and . allowed)")
        return Response(content=f"{callback}({json_string});", media_type="application/javascript")

[Copilot]

List support was added to wrap_jsonp (serializing list via json.dumps), but the existing unit tests for wrap_jsonp (see openlibrary/tests/fastapi/test_models.py) don’t cover the list case. Please add a test that passes a list (with and without a callback) to prevent this regression from reappearing.

[jimchamp]

@copilot apply changes based on this feedback

[Copilot]

Implemented in 81fbb53. I added regression tests in openlibrary/tests/fastapi/test_models.py covering list payloads for both cases: no callback (application/json) and with callback (application/javascript).

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• api-inference.huggingface.co
  • Triggering command: /opt/java/openjdk/bin/java /opt/java/openjdk/bin/java -server -Xms512m -Xmx512m -XX:+UseG1GC -XX:+PerfDisableSharedMem -XX:+ParallelRefProcEnabled -XX:MaxGCPauseMillis=250 -XX:+UseLargePages -XX:+AlwaysPreTouch -XX:+ExplicitGCInvokesConcurrent -Xlog:gc*:file=/var/solr/logs/solr_gc.log:time,uptime:filecount=9,filesize=20M -Dsolr.jetty.inetaccess.includes= -Dsolr.jetty.inetaccess.excludes= -Dsolr.log.dir=/var/solr/logs -Djetty.port=8983 -DSTOP.PORT=7983 -DSTOP.KEY=solrrocks -Djava.io.tmpdir=/tmp -Duser.timezone=UTC (dns block)
• api.mistral.ai
  • Triggering command: /opt/java/openjdk/bin/java /opt/java/openjdk/bin/java -server -Xms512m -Xmx512m -XX:+UseG1GC -XX:+PerfDisableSharedMem -XX:+ParallelRefProcEnabled -XX:MaxGCPauseMillis=250 -XX:+UseLargePages -XX:+AlwaysPreTouch -XX:+ExplicitGCInvokesConcurrent -Xlog:gc*:file=/var/solr/logs/solr_gc.log:time,uptime:filecount=9,filesize=20M -Dsolr.jetty.inetaccess.includes= -Dsolr.jetty.inetaccess.excludes= -Dsolr.log.dir=/var/solr/logs -Djetty.port=8983 -DSTOP.PORT=7983 -DSTOP.KEY=solrrocks -Djava.io.tmpdir=/tmp -Duser.timezone=UTC (dns block)
• langchain4j.dev
  • Triggering command: /opt/java/openjdk/bin/java /opt/java/openjdk/bin/java -server -Xms512m -Xmx512m -XX:+UseG1GC -XX:+PerfDisableSharedMem -XX:+ParallelRefProcEnabled -XX:MaxGCPauseMillis=250 -XX:+UseLargePages -XX:+AlwaysPreTouch -XX:+ExplicitGCInvokesConcurrent -Xlog:gc*:file=/var/solr/logs/solr_gc.log:time,uptime:filecount=9,filesize=20M -Dsolr.jetty.inetaccess.includes= -Dsolr.jetty.inetaccess.excludes= -Dsolr.log.dir=/var/solr/logs -Djetty.port=8983 -DSTOP.PORT=7983 -DSTOP.KEY=solrrocks -Djava.io.tmpdir=/tmp -Duser.timezone=UTC (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[RayBB]

Thank for fixing this. Tested locally and confirmed this does fix the issue!