[BUG]: It doesn't seem like octokit rest returns a RequestError object

[shubhbapna]

What happened?

I have function which gets some repository details and does some stuff with it. It's enclosed in try catch block to handle different errors. Here is a simplified version of it

async function doSomething(owner: string, repo: string) {
  try {
    const octokit = new Octokit();
    await octokit.repos.get({
      owner,
      repo,
    });
  } catch (err) {
    if (err instanceof RequestError) {
      console.log("octokit error")
    } else {
      console.log("non octokit error")
    }
  }
}

Although after running my code with non-existant owner and repo, this function always logs "non octokit error" instead of "octokit error"

However according to this discussion all the error objects should be an instance of RequestError: octokit/octokit.js#2039

Versions

@octokit/rest: ^18.12.0
Node: v16.19.0

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[dbalatero]

I'm having the same issue, error handling is really underdocumented in octokit.

import { RequestError } from "@octokit/request-error";

try {
  // some octokit request
} catch (error) {
  if (error instanceof RequestError) {
    console.log("this never happens");
  }
}

Can the docs be made explicit what to do here? I'm assuming I'm importing the wrong RequestError, but the documentation leads me to do what I'm doing in my code sample above:

• There is no explicit section on error catching I've found in octokit
• The docs explicitly talk about this @octokit/request-error package, which tricks me into thinking I should add it to my project and use it

[margueritepd]

I had the same issue. I resolved it by importing everything from octokit, not @octokit/rest.

import { RequestError, Octokit } from 'octokit'

Perhaps this library needs to re-export the RequestError class from its @octokit/request dependency, so we can reference the exact same class that @octokit/rest is creating its request call raises an exception? - Actually it looks like @octokit/request would also need to re-export it, since it's not currently exported from that lib.

Caveat: I'm a node noob so that might not be quite right

[dbalatero]

Also I'd love to help contribute documentation or changes via PR, but since things aren't documented I'll need a project maintainer to weigh in here on the right changes to make! What's the best way to get some 👀 on this?

[wolfy1339]

The RequestError class is indeed from @octokit/request-error.

Feel free to test things out locally to see what works and come back with a PR.

I don't understand myself why it doesn't match

[wolfy1339]

We catch all errors and wrap them in RequestError if they aren't of that type

https://github.com/octokit/request.js/blob/02b572356bf664662204f76c53b533bb9b7f9d4f/src/fetch-wrapper.ts#L130-L150

[G-Rath]

Using this in both CommonJS and ESM files gives the expected result:

import { Octokit } from '@octokit/rest';
import { RequestError } from '@octokit/request-error';

async function doSomething(owner, repo) {
  try {
    const octokit = new Octokit();
    await octokit.repos.get({
      owner,
      repo,
    });
  } catch (err) {
    if (err instanceof RequestError) {
      console.log("octokit error")
    } else {
      console.log("non octokit error")
    }
  }
}

doSomething().then(console.log);

workspace/projects-scrap/octo-test is 📦 v1.0.0 via  v20.6.1
❯ node file.cjs
octokit error
undefined

workspace/projects-scrap/octo-test is 📦 v1.0.0 via  v20.6.1
❯ node file.mjs
octokit error
undefined

We need more information about the environment you're running - instanceof requires the same underlying class/prototype which isn't always the case e.g. if you're using vm.

[dbalatero]

I wonder if running in a monorepo via Turborepo where multiple sub-packages are compiled and imported could cause a mismatch in class/prototypes?