Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BUG: .run() throws D1 Error in cloud #441

Closed
AlexBlokh opened this issue Nov 18, 2022 · 4 comments 路 Fixed by #480
Closed

馃悰 BUG: .run() throws D1 Error in cloud #441

AlexBlokh opened this issue Nov 18, 2022 · 4 comments 路 Fixed by #480
Assignees
Labels
behaviour mismatch Different behaviour to Workers runtime bug Something isn't working quick win
Milestone

Comments

@AlexBlokh
Copy link

AlexBlokh commented Nov 18, 2022

What version of Wrangler are you using?

2.4.2

What operating system are you using?

Mac

Describe the Bug

await env.DB.prepare('insert into users (name, email) values (?, ?) returning *').bind(name, email).run();

throws Exception in cloud

"exceptions": [
    {
      "name": "Error",
      "message": "D1_ERROR",
      "timestamp": 1668795822085
    }
  ]

These three below work as expected in cloud

await env.DB.prepare('insert into users (name, email) values (?, ?) returning *').bind(name, email).all();
await env.DB.prepare('insert into users (name, email) values (?, ?) returning *').bind(name, email).first();
await env.DB.prepare('insert into users (name, email) values (?, ?) returning *').bind(name, email).raw();

All four requests do work as expected locally

@AlexBlokh AlexBlokh added the bug Something isn't working label Nov 18, 2022
@Skye-31
Copy link
Contributor

Skye-31 commented Nov 20, 2022

According to the docs, .run() should be used for queries that return no results. While it probably shouldn't be a generic error - it should be returning an error.

@AlexBlokh
Copy link
Author

It may or may not fail, it's completely up to the API design, at least should be consistent locally and in the cloud

@Skye-31
Copy link
Contributor

Skye-31 commented Nov 21, 2022

Agreed

@penalosa
Copy link
Contributor

Hi鈥攖hanks for reporting this! That's definitely a local/remote behaviour mismatch that we should fix. In the meantime, you can often get more helpful error message by checking error.cause (in this case the error message is ERROR 9007: SQL execute error: Execute returned results - did you mean to call query?:

export default {
  async fetch(
    request: Request,
    env: Env,
    ctx: ExecutionContext
  ): Promise<Response> {
    await env.DB.exec(
      "create table if not exists users (name text, email text);"
    );
    try {
      await env.DB.prepare(
        "insert into users (name, email) values (?, ?) returning *"
      )
        .bind("a", "b")
        .run();
    } catch (e) {
      return new Response(e.cause);
    }
    return new Response("Hello World!");
  },
};

@penalosa penalosa transferred this issue from cloudflare/workers-sdk Nov 22, 2022
@penalosa penalosa added the behaviour mismatch Different behaviour to Workers runtime label Nov 22, 2022
@mrbbot mrbbot self-assigned this Nov 28, 2022
@mrbbot mrbbot modified the milestones: 2.11.0, 2.12.0 Dec 22, 2022
mrbbot added a commit that referenced this issue Jan 23, 2023
Previously, Miniflare had its own implementations of `BetaDatabase`
and `Statement`. These were subtly different to the implementations
in the D1 Wrangler shim D1JS, causing behaviour mismatches.

This change switches the implementation to use the shim, with
Miniflare implementing the underlying D1 HTTP API instead. We'll
need to do this anyway when adding D1 support to Miniflare 3 using
`workerd`.

Specific changes:
- Throw when calling `D1PreparedStatement#run()` with statements that
  return data, closes #441
- Fix response envelope format, closes #442 and
  cloudflare/workers-sdk#2504
- Fix binding/return of `BLOB`-typed values, closes wrangler2#2527
- Fix `D1Database#raw()` return, closes cloudflare/workers-sdk#2238
  (already fixed in #474)
- Add support for `D1Database#dump()`
- Run `D1Database#{batch,exec}()` statements in implicit transaction
- Only run first statement when calling
  `D1PreparedStatement#{first,run,all,raw}()`
mrbbot added a commit that referenced this issue Jan 23, 2023
Previously, Miniflare had its own implementations of `BetaDatabase`
and `Statement`. These were subtly different to the implementations
in the D1 Wrangler shim D1JS, causing behaviour mismatches.

This change switches the implementation to use the shim, with
Miniflare implementing the underlying D1 HTTP API instead. We'll
need to do this anyway when adding D1 support to Miniflare 3 using
`workerd`.

Specific changes:
- Throw when calling `D1PreparedStatement#run()` with statements that
  return data, closes #441
- Fix response envelope format, closes #442 and
  cloudflare/workers-sdk#2504
- Fix binding/return of `BLOB`-typed values, closes wrangler2#2527
- Fix `D1Database#raw()` return, closes cloudflare/workers-sdk#2238
  (already fixed in #474)
- Add support for `D1Database#dump()`
- Run `D1Database#{batch,exec}()` statements in implicit transaction
- Only run first statement when calling
  `D1PreparedStatement#{first,run,all,raw}()`
@mrbbot mrbbot closed this as completed in 2d0fbc6 Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behaviour mismatch Different behaviour to Workers runtime bug Something isn't working quick win
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants