weave icon indicating copy to clipboard operation
weave copied to clipboard

fix(weave_ts): extract meaningful errors when calls to the traceServerApi fail (#4487)

Open pjlsergeant opened this issue 7 months ago • 4 comments

Description

Adds debugging messages of last-resort to HTTP Responses thrown as errors inside src/weaveClient.ts. This is obviously a band-aid patch: the better fix would be to lobby the swagger-typescript-api not to throw a Response object, and to also add proper error handling to each of the call sites in src/weaveClient.ts. HOWEVER, the Node SDK is essentially unusable without this in its current form (see: #4487).

Errors before with no indication of where the error occurred or what the error was:

node:internal/process/promises:392
      new UnhandledPromiseRejection(reason);
      ^

UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "#<_Response>".
    at throwUnhandledRejectionsMode (node:internal/process/promises:392:7)
    at processPromiseRejections (node:internal/process/promises:475:17)
    at process.processTicksAndRejections (node:internal/process/task_queues:106:32) {
  code: 'ERR_UNHANDLED_REJECTION'
}

Errors after:

Error: Trace Server API returned 403 for https://trace.wandb.ai/obj/create: {"reason":"Invalid object name: models-gpt-4o-mini+o3-mini. Contains invalid characters: ['+']. Please upgrade your `weave` package to `>0.51.0` to prevent this error."}
    at <anonymous> (/Users/pjlsergeant/dev/weave/sdks/node/src/weaveClient.ts:874:13)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async <anonymous> (/Users/pjlsergeant/dev/weave/sdks/node/src/weaveClient.ts:639:24)

Testing

I ran the test suite, it passed.

pjlsergeant avatar May 14 '25 13:05 pjlsergeant

This PR requires manual approval from a wandb user to run all CI checks.

To see the current diff, click here.

To approve CI for this PR as of this commit, comment:

/approve fa183a4bd4dc7edc7a981a3a3128d4fbf90aaddb

circle-job-mirror[bot] avatar May 14 '25 13:05 circle-job-mirror[bot]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

github-actions[bot] avatar May 14 '25 13:05 github-actions[bot]

I have read the CLA Document and I hereby sign the CLA

pjlsergeant avatar May 14 '25 13:05 pjlsergeant

hi @pjlsergeant

Thanks for making the change.

I think a more universal approach is to directly log an error in the traceServerApi.ts. It looks like swagger-typescript-api supports generating a template first, having the template manually modified, then generate the eventual ts file using the modified template. We should be able to modify the template to add logging. That seems to be the cleanest approach available.

Please let me know if you are willing to go this route. Otherwise, feel free to leave this to our team(Weave) to handle it.

I know debugging the unhandled rejection is such a pain. Have you tried capturing unhandled rejections on the process level?

process.on('unhandledRejection',  callback);

would it be used as a temporary solution at the moment?

chance-wnb avatar Jul 07 '25 22:07 chance-wnb