denoland / deploy_feedback

For reporting issues with Deno Deploy
https://deno.com/deploy
74 stars 5 forks source link

[Bug]: Getting different output in local CLI and Deno Deploy #648

Closed ip2location closed 5 months ago

ip2location commented 6 months ago

Version: Deno 1.42.1

We have developed a Deno module for our IP2Location BIN database. One of our users complained that he was unable to query the data from our IP2Location BIN database using our module. After a closer examination and debugging of the the code, we found out that the node:fs.readSync is not working correctly and causing our module to return wrong data. Below is the simplest code to reproduce the outputs:

import { Application, route, Router } from "https://deno.land/x/oak/mod.ts";
import { oakCors } from "https://deno.land/x/cors/mod.ts";
import { readSync, openSync, closeSync, existsSync, statSync } from "node:fs";
import { Buffer } from 'node:buffer';

function readRow(fd, readBytes, position) {
  let buffer = new Buffer.alloc(readBytes);
  console.log("length " + buffer.length);
  let totalRead = readSync(fd, buffer, 0, readBytes, position - 1);
  console.log("readBytes: " + readBytes);
  console.log("totalRead: " + totalRead);
  return buffer;
}

function read32Row(position, buffer) {
  let var1 = buffer.readUInt32LE(position);
  console.log("var1: " + var1);
  return var1;
}

router.get('/', ctx => {

    const ip = ctx.request.ip;
    const mesg = 'Calling from local ipl deno';

    let fd = openSync("./IP2LOCATION-LITE-DB3.BIN", "r");
    let len = 64; // 64-byte header
    let row = readRow(fd, len, 1);
    let dbCount = read32Row(5, row);
    console.log("dbCount: " + dbCount);
    let indexBaseAddress = read32Row(21, row);
    console.log("indexBaseAddress: " + indexBaseAddress);
    row = readRow(fd, 524288, indexBaseAddress);
    console.log("read32Row result for position 524284 is " + read32Row(524284, row));

    ctx.response.body = {
        ip: ip,
        mesg: mesg
    };
})

app.use(oakCors());
app.use(router.routes());

app.listen({ port: 8000 });

The expected output is as below(Running in local): denodeploy2

However, in Deno Deploy, I got a different output like this(Sequence from bottom to top): denodeploy3

The IP2Location LITE DB3 BIN database used in the code can be obtained by signing up from here: https://lite.ip2location.com/database/db3-ip-country-region-city.

Can we get someone to take a look at this? Thanks.

kewp commented 5 months ago

hi @ip2location it looks like the issues is the file read command - if you look at this endpoint https://kewp-refer-location.deno.dev/test you can see bytes read is only 6912 when we asked for 524288! maybe this is a limitation on deno deploy (limited file read bytes), not sure. i've updated my test repo with the code used for the endpoint https://github.com/kewp/refer-location

note: i switched over to standard buffers to check if that was the issue ... no dice

kewp commented 5 months ago

interestingly - if you refresh https://kewp-refer-location.deno.dev/test you'll see the number of bytes read changes!

kewp commented 5 months ago

@ip2location i've fixed it. if you read the file all in one go then you can read the bytes in correctly. see https://kewp-refer-location.deno.dev/test2 the updated code is in https://github.com/kewp/refer-location

marvinhagemeister commented 5 months ago

This seems to be an issue with Deno Deploy. Transferring it to the Deploy issue tracker.

lucacasonato commented 5 months ago

This is either a lack of documentation in Node (and you need to fix your code to call read in a loop until readBytes >= length), or it's a bug in Deno. Trying to determine which right now in this issue: https://github.com/nodejs/node/issues/52447

The reason you are seeing different behaviour in the Deno CLI and Deno Deploy is that the FS have different speeds locally and on Deploy, so different number of bytes can be read with a single syscall.

lucacasonato commented 5 months ago

Ok, I have received a response on the Node issue. It looks like this is not a Deno bug, but instead stems from how the Linux read syscall works.

When you call read() it is not required that it fills up the entire buffer, even if data is available. This means you have to inspect the readBytes returned value and check whether enough bytes have been read - if you have not read enough bytes, you must call read() again (offset to readBytes, with length = length - readBytes), until you have as many bytes as you need.

The Deno docs for the file system read method document this behaviour: https://deno.land/api@v1.42.1?s=Deno.FsFile&p=prototype.read.

The reason you did not encounter this locally, is that the chance for these "partial reads" depends on the specific file system you are using. On Deploy they are more likely than locally.

kewp commented 5 months ago

Thanks @lucacasonato that makes a lot of sense. Didn't catch that comment on the Deno docs

ip2location commented 5 months ago

We will close the ticket now as we found a workaround to the issue. However it's still strange that the nodeJs FS module can behave differently in local and Deno Deploy.