Recording Exceptions Example for Typescript Doesn't Work Out of the Box

[Grunet]

The example for "Recording exceptions" for Typescript doesn't work as-is. You have to do something to convince the compiler that you're allowed to pass the ex (typed as unknown by default since I think JS lets you throw anything) into the recordException call.

https://opentelemetry.io/docs/languages/js/instrumentation/#recording-exceptions

Typing it as any in the catch block or explicitly casting it as Error in the recordException call both seem to work.

Not sure if this is the right code but it seems like recordException is not making any assumptions about the input type anyways, so it seems safe to do these sorts of things.

https://github.com/open-telemetry/opentelemetry-js/blob/84cce753617a1195138cab3254e1b22f80982150/packages/opentelemetry-sdk-trace-base/src/Span.ts#L317

[Grunet]

Definitely the tiniest of papercuts, but I figured I'd mention it while I was here

[svrnm]

@Grunet thanks for reporting! All feedback is helpful

@open-telemetry/javascript-approvers PTAL!

[cartermp]

Huh, I get (local var) ex: any when I do a snippet like this:

const doWork = (x: number) => {
  if (x > 0) {
    throw new Error("uh oh! failed to do the thing");
  }

  return x;
}

//...

try {
  doWork(1);
} catch (ex) {
  span.recordException(ex);
  span.setStatus({ code: SpanStatusCode.ERROR });
}

[cartermp]

@Grunet do you have a copy-pastable repro per chance?

[Grunet]

@Grunet do you have a copy-pastable repro per chance?

It's not an exact repro, but I just copied recordException's type signature and the types it was depending on and put it into this playground that exhibits the issue for me

https://www.typescriptlang.org/play/?#code/PTAEHUFMBsGMHsC2lQBd5oBYoCoE8AHSAZVgCcBLA1UABWgEM8BzM+AVwDsATAGiwoBnUENANQAd0gAjQRVSQAUCEmYKsTKGYUAbpGF4OY0BoadYKdJMoL+gzAzIoz3UNEiPOofEVKVqAHSKymAAmkYI7NCuqGqcANag8ABmIjQUXrFOKBJMggBcISGgoAC0oACCbvCwDKgU8JkY7p7ehCTkVDQS2E6gnPCxGcwmZqDSTgzxxWWVoASMFmgYkAAeRJTInN3ymj4d-jSCeNsMq-wuoPaOltigAKoASgAywhK7SbGQZIIz5VWCFzSeCrZagNYbChbHaxUDcCjJZLfSDbExIAgUdxkUBIursJzCFJtXydajBBCcQQ0MwAUVWDEQC0gADVHBQGNJ3KAALygABEAAkYNAMOB4GRonzFBTBPB3AERcwABS0+mM9ysygc9wASnJjSpcPgYrIiV5StW+X67EQ0m+Op5AD5QABvRQlBGgC2gZ0ABgdbpKJSy8Ak-UgYZpZDYZCVfPYmngmAAhDiGJjIDEMNwMF8BJxmHydQBud2gAC+wRKTlQ+K8q1LlZCARbwVQZDwrrLOZN8SVAEYS4py6NUBovWsA2WnAgyNw6RZqA1OErJ43gskuLB6o1QDPxfPVoudyu1sfl1aF5Al40dVadPAKK43U3UO1QFeb15uWWAD4fo9rxPcBdgAYXgbglBKf9P2A3YAFl9EBZgoNAGDAK-EDYgAOQZVD-ypSgC1LRQMgUMhkgYJZYOXLDMHAyCuxKBBIKtQjhjQ61bW+UsSk4PCAH42PbYZeNAZBBGQyAhKuETiLLKkqPiGT2PkpsyO+SjqIwuDYkQySGBQpi0UglS5JGf9OBtO0yDEiSpOEojmDE-jkDMpyxMU2BlMc0Th2CDSKKolAaMaOjcOQYyWOk3yC04qzuNsst7MMmLZI8stXMgWLnIU1AlPcvymxUHBmg8MgvEQcVnGBdgaDzRgC3YVL+FgaB1ESDl4D0EQvD5OkGSZQQ+SSbE+XABxUAAcmEbCIz5IIVAAeS+Mh3kESB+BQo58rIBRXGkTsnCqnQOK+Da0UQaFhEuPMJHFaIRAMDhsQWJhWA4HgghKIA

If you need a full runnable minimal repro just lmk, it just might take a sec to put together

[svrnm]

@open-telemetry/javascript-approvers please take a look

[pichlermarc]

Ah yes this worked in older typescript versions where anything that was caught was always any which can be assigned to everything. That can cause some not-so-fun errors, since one can throw whatever one likes in JavaScript.

I think since TypeScript 4.4, if you set your TypeScript settings to strict: true it will use unknown instead - that's what's happening here.

So we may want to change the example like so

 import opentelemetry, { SpanStatusCode } from '@opentelemetry/api';
 
 // ...
 
 try {
   doWork();
 } catch (ex) {
-  span.recordException(ex);
+  if(ex instanceof Error) {
+    span.recordException(ex);
+  }
   span.setStatus({ code: SpanStatusCode.ERROR });
 }

(note: it may also be worth to add the same check to the JS example since it will otherwise just try to record something that may not be an error)

[svrnm]

thanks @pichlermarc !

@Grunet would you be interested in fixing this?

[cartermp]

Aha, that explains why I couldn't reproduce. I had an older Typescript in my test repo!

[Grunet]

thanks @pichlermarc !

@Grunet would you be interested in fixing this?

Sure just updating the docs to add the instanceof check? Yeah I can do that