Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

Conversation

@dr-skot
Copy link

@dr-skot dr-skot commented Sep 10, 2021

… throw errors instead of console.warn

Love how nifty & succinct this is.

I just added { date: new Date(), offset: 0 } to the return value if there's an error fetching,
(so for example it won't break in server-side-rendering scenarios, where client code might run on the server)

and added an option to rethrow errors instead of swallowing them with console.warn.

Thanks for the good code!

@dr-skot
Copy link
Author

dr-skot commented Sep 10, 2021

... report errors

I realized I put the rethrow inside the loop, thus punting at the first sign of trouble!
Of course it's possible that just one or two fetches fail out of 10, as illustrated in your test scenario.
So I changed my new option to withErrors. If you set it to true you get extra return-value properties:

  • errors an array of errors that were thrown, if any
  • fetchCount: the number of fetches attempted (ie 10, in the current implementation)

You can compare errors.length to fetchCount to see if any of the fetches succeeded at all.

Sorry for two in a row! Thanks again.

@NodeGuy
Copy link
Owner

NodeGuy commented Jul 3, 2023

For your use case of server-side-rendering scenarios it's better to use a wrapper than to pollute getServerDate with local time, e.g.:

const wrapper = async () => {
  const serverDate = await getServerDate();

  return {
    date: new Date(),
    offset: 0,
    ...serverDate.date,
  };
};

For error reporting you can again use a wrapper which I prefer to keep getServerDate small:

const wrapper2 = async () => {
  const exceptions = [];

  const fetchSample = async () => {
    try {
      return fetchSampleImplementation();
    } catch (exception) {
      exceptions.push(exception);
      throw exception;
    }
  };

  const serverDate = await getServerDate({ fetchSample });
  return { ...serverDate, exceptions };
};

I'll export fetchSampleImplementation to make this easier.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants