hirosystems / stacks.js

JavaScript libraries for identity, auth, storage and transactions on the Stacks blockchain.
https://stacks.js.org
MIT License
944 stars 307 forks source link

`dangerouslyIgnoreEtag` is ignored when calling putFile #1632

Closed pradel closed 4 months ago

pradel commented 4 months ago

What version of Stacks.js are you using?

"@stacks/auth": "6.11.3", "@stacks/blockchain-api-client": "7.8.1", "@stacks/common": "6.10.0", "@stacks/connect": "7.7.0", "@stacks/connect-react": "22.4.1", "@stacks/encryption": "6.11.3", "@stacks/keychain": "4.3.8", "@stacks/network": "6.11.3", "@stacks/profile": "6.11.3", "@stacks/storage": "6.11.3", "@stacks/transactions": "6.11.3", "@stacks/wallet-sdk": "6.11.3",

Describe the bug

Since the migration to the new Gaia infra (don't know if it's related tho) we have had many reports of users having a 412 HTTP error code when trying to write to a Gaia bucket. I finally got the error so I was able to see what's happening there and it seems that the dangerouslyIgnoreEtag flag when calling putFile is not respected. When I do the call with dangerouslyIgnoreEtag set to true I see the following response from the Gaia server.

{
    "message": "The provided ETag does not match that of the resource on the server",
    "error": "PreconditionFailedError"
}

For the following call

await storage.putFile(
    publicStoriesFileName,
    JSON.stringify(publickStoriesFile),
    {
      encrypt: false,
      // It's safe to use this flag here as the public index is built from the private index
      // If the private index is outdated the code will throw an error when we write the storiesFileName
      // before this call.
      dangerouslyIgnoreEtag: true,
    },
  );

Request Headers:

Screenshot 2024-02-19 at 23 08 07

Response Headers:

Screenshot 2024-02-19 at 23 07 07

How to reproduce

Not sure how to reproduce consistently, many different people are facing that issue on Sigle tho.

Expected behavior

Using dangerouslyIgnoreEtag should make the call pass.

pradel commented 4 months ago

A note that this part of the code worked for 2+ years and we started to get reports at the time of the Gaia Hub migration.

janniks commented 4 months ago

@hirosystems/devops Can you check on this, or lmk how to best debug the Gaia config / proxies, that could impact it?

CharlieC3 commented 4 months ago

@janniks Can you describe how setting dangerouslyIgnoreEtag affects the request made to hub.blockstack.org?

janniks commented 4 months ago

@pradel to double-check, is the screenshot/request of a 412 with dangerouslyIgnoreEtag: true?

janniks commented 4 months ago

@CharlieC3

  if (!dangerouslyIgnoreEtag) {
    if (newFile) {
      headers['If-None-Match'] = '*';
    } else if (etag) {
      headers['If-Match'] = etag;
    }
  }

Is how headers are set. So only setting headers if dangerouslyIgnoreEtag: false.

Is etag handled by a proxy/cloudflare or done directly by the hub? Trying to figure out if there's a Stacks.js bug, but haven't been able to consistently reproduce yet..

pradel commented 4 months ago

@janniks yeah it's the screenshot from the failing call, it's weird that the If-Match header is set

CharlieC3 commented 4 months ago

From what I can discern, the failure is coming from the origin (the CDN) and not from Cloudflare. So it doesn't appear to be related to the infra changes from last year since there's been no changes with the CDN itself. As @pradel pointed out, the inclusion of the If-Match header in the request does not seem expected if dangerouslyIgnoreEtag is properly set to true, and I'm wondering if this could be what's causing the 412's to happen.

janniks commented 4 months ago

Thanks 🙏 Yeah, from the code I don't think that should be possible -- (I don't think If-Match headers are ever set by the browser/runtime automatically either). @pradel can you tell for certain which line it's coming from? e.g. could it be a different call? https://github.com/sigle/sigle/blob/3635e271720eb766d208325d3548699893e38157/apps/sigle/src/utils/index.ts#L34-L38C9

pradel commented 4 months ago

@janniks Yeah that's strange might be the wrong screenshot, during my tests I added the option to all the put calls made to Gaia. Ofc I don't have the error anymore today otherwise it would be too simple to debug. Will have to wait till the error is popping again for me. I will add the dangerouslyIgnoreEtag to all the calls made on Sigle and push to prod to see if that helps to see if I get fewer reports.

janniks commented 4 months ago

lmk how that goes -- happy to reopen