Triggered "FIX ME! variant { ok; err : text } <: opt principal via special opt rule."

[ByronBecker]

I was upgrading a canister and ran into this issue.

% dfx deploy cycleops                              
Deploying: cycleops
All canisters have already been created.
Building canisters...
Shrink WASM module size.
Installing canisters...
FIX ME! variant { ok; err : text } <: opt principal via special opt rule.
This means the sender and receiver type has diverged, and can cause data loss.
Upgrading code for canister cycleops, with canister ID sp3hj-caaaa-aaaaa-aaajq-cai
Deployed canisters.

Prior to the upgrade, this is the code that was changed.

  public shared ({ caller }) func stopTopupTimerCanister() : async ?Principal {
    if (not Admins.isAdmin(admins, caller)) return null

    switch (topupTimerActor) {
      case (?timer) {
        let principal = Principal.fromActor(timer);
        await ManagementCanisterActions.stopCanister(principal);
        ?principal
      };
      case null { null };
    };
  };

Became

  public shared ({ caller }) func stopTopupTimerCanister() : async Result.Result<(), Text> {
    if (not Admins.isAdmin(admins, caller)) return #err("Not authorized");

    switch (topupTimerActor) {
      case (?timer) {
        let principal = Principal.fromActor(timer);
        await ManagementCanisterActions.stopCanister(principal);
        #ok
      };
      case null { #err("No TopupTimer canister exists") };
    };
  };

[ByronBecker]

Note that this bug was found in the context of trying to solve https://github.com/dfinity/motoko/issues/3857 🙂

[chenyan-dfinity]

I don't see a problem here. You are changing the return type, which is a breaking change. Maybe we should error out in install instead of a warning?

[ByronBecker]

I don't argue that the change is breaking, but the "FIX ME!" comment/error messaging feedback feels like a todo that wasn't handled.

Instead I'd expect this to show up like all other breaking changes, where dfx would tell me that I'm making a breaking change, and prompt me enter "yes" to force through the upgrade

[chenyan-dfinity]

Yeah, I agree. FIX ME is a warning to the user to fix the "almost" breaking change. Because you are returning an option type, result from the old version always becomes null with the new client, so it's technically not a breaking change, but nice to fix. I will file a ticket to promote this warning to error in dfx, so that users can enter "yes".

[sesi200]

Tracked internally under SDK-1024