[BREAKING CHANGE] Disconnect/connect (for relations) throws errors needlessly

[jhalborg]

Problem

Currently when you try to connect or disconnect relationships, you need to be needlessly cautious that you haven't done it before, otherwise the command will throw.

It's weird that it throws, seeing as the state of the data afterwards is as the request wanted it in the first place anyway.

Here's an example:

await prisma.user.update({
    where: {
      id: 'u123',
    },
    data: {
      Post: {
        connect: {
          id: 'p123',
        },
      },
    },
  })

// will throw
await prisma.user.update({
    where: {
      id: 'u123',
    },
    data: {
      Post: {
        connect: {
          id: 'p123',
        },
      },
    },
  })

The same applies for disconnect.

Additional Context: https://prisma.slack.com/archives/CKQTGR6T0/p1568120850240700?thread_ts=1567958862.225000&cid=CKQTGR6T0

Suggested Solution

Don't throw. Allow connects and disconnects to either apply the change or ignore the action.

[pantharshit00]

Current behaviour is photon will also throw if you try to connect to a already existing connected node.

We might need a product decision here on what we want the actual behaviour to be.

[RafaelKr]

When finally deciding what the behavior should be, you should also take into consideration how Photon will ensure a working connection when running on serverless infrastructure, especially after a frozen function is resumed. In the best case this just happens automatically.

Besides that I also wouldn't expect it to throw. As stated by @jhalborg the requested state is the current state.

Otherwise I currently can't think of a real case where a developer has to call connect when he is already connected to a database. Only with bad state management or if a disconnect has failed somehow.

Nevertheless my opinion is that Photon shouldn't throw but instead log a warning which informs the developer that a call to connect/disconnect would not have been needed.

[mavilein]

This not a bug as this is the intended behavior. Therefore marking it as a feature request. @sorenbs is the owner of the spec for this API. He should make a call whether we want to change this behavior.

[jhalborg]

@RafaelKr and @mavilein - I think you've misread the issue :-) I didn't mean connects/disconnects from the client to the database, but between related nodes in the data 😄 See the Slack link for more context

[mavilein]

@jhalborg : I did not. My comment still stands. This was indeed intentional. @sorenbs will look into this and make a call. :-)

[jhalborg]

@mavilein Ah, OK - only @RafaelKr , then :-)

I'll just reiterate that I'd find it very unintuitive to have to handle an error for an operation that leads to the result I wanted from the operation :-)

[RafaelKr]

@jhalborg You are right, I have indeed not paid enough attention and misunderstood it.

But still I have the same opinion for connecting/disconnecting relations: It shouldn't throw an error and in this case even no warning.

I don't know what exactly the Prisma Query Engine is doing, but I think when doing the same operation in raw SQL it also wouldn't throw an error. Am I right about this?

Maybe a comparable situation of real life that just came to my mind is that:
I ask my colleague "Could you please plug in/out my charger cable?", he replies "Okay", then he sees it's already plugged in/out and does nothing.

[janpio]

Internal Note: This might make sense, but we don't have time to think about it right now.

[jhalborg]

@janpio - Wouldn't it make sense to spend time on this before GA of the client, as it would otherwise lead to a lot of error-handling code for users of the library that shouldn't be necessary, and that would have to be removed if this is implemented later on? I think it'll save your users a lot of code, time and confusion.

[janpio]

Certainly, but we can not do all the things that would make sense right now, so we have to make decisions where to focus on. Fixing real bugs and finishing functionality that is needed currently is higher on that list. But we will get to this.

[matthewmueller]

Hey @jhalborg, I re-wrote your initial problem statement. I hope you don't mind!

[jkb-kt]

When can we expect this done, please?

[matthewmueller]

Next Step:

• Check that this works

[matthewmueller]

This works now, if you try to connect an already connected relation, it's a no-op.