Fix attachment streaming errors and add observability for failed attachments by gasperzgonec · Pull Request #136 · devrev/adaas-sdk

gasperzgonec · 2026-01-29T16:29:46Z

Description

This PR fixes the following issues:

Attachments of size 0 failing
Added more information for attachments that failed to upload

Connected Issues

#ISS-247801

Checklist

Tests added/updated and ran with npm run test OR no tests needed.
Ran backwards compatibility tests with npm run test:backwards-compatibility.
Code formatted and checked with npm run lint.
Tested airdrop-template linked to this PR.
Documentation updated and provided a link to PR / new docs OR no docs needed.

Before, some error messages contained [object Object], so I had to add JSON.stringify(<error>) to fix that. I changed `. "+error` to `.", error`, which automatically adds a space and stringifies the error.

src/uploader/uploader.ts

src/types/extraction.ts

src/multithreading/worker-adapter/worker-adapter.ts

src/attachments-streaming/attachments-streaming-pool.ts

…tion.

src/attachments-streaming/attachments-streaming-pool.ts

src/multithreading/worker-adapter/worker-adapter.ts

radovanjorgic · 2026-02-10T05:28:15Z

src/multithreading/worker-adapter/worker-adapter.ts

+          return {
+            error: {
+              message:
+                `Error while streaming to artifact for attachment ID ${attachment.id}. Skipping attachment. ` +
+                JSON.stringify(serializeError(uploadedArtifactError)),
+              fileSize: fileSize,
+            },
+          };


Let's think of solving this in serializeError or somewhere else (maybe even in emit - if string passed just forward, if not stringify and forward). Correct me if I am wrong but you stringified here (and on few other places) because ErrorRecord interface requires message to be a string.

I changed how the serializeError works; it now returns strings no matter what. If it's JSON error, it stringifies it, so that we can remove the JSON.stringify calls everywhere.

Can you check this? I remember we had issues in past when stringifying errors. Were getting empty objects as output sometimes and it was hard to debug.

radovanjorgic · 2026-02-10T05:28:41Z

src/uploader/uploader.ts

    );
    if (fileError) {
      return {
        error: new Error(


Can you check if we need to wrap this in Error?

This has to be an error, as we're returning Promise<UploadResponse>, which contains a field error, which is optional, but is of type ErrorRecord:

/** * UploadResponse is an interface that defines the structure of the response from upload through Uploader. */ export interface UploadResponse { artifact?: Artifact; error?: ErrorRecord; } /** * ErrorRecord is an interface that defines the structure of an error record. */ export interface ErrorRecord { message: string; }

Yes but ErrorRecord is type defined by us, while Error is generic error constructor in JavaScript (which coincidentally has message field).

radovanjorgic · 2026-02-10T05:36:09Z

src/uploader/uploader.ts

  ): Promise<UploaderResult<ArtifactToUpload>> {
    const url = `${this.devrevApiEndpoint}/internal/airdrop.artifacts.upload-url`;

+    if (fileSize != null && fileSize! <= 0) {


So you actually made fileSize required parameter? I am not sure if we should do that because s3interact accepts size not to be specified in first this request if I remember correctly, but please check.

I would rather do sth like if(fileSize && fileSize >= 0) { return error }.

fileSize is not required. What this check is doing, is that if the fileSize is provided and it's either 0 or negative, we throw an error, as the fileSize cannot be negative and a size of 0 isn't allowed.

Okay so we can check it with this only then: if(fileSize! <= 0) {return error}.

src/attachments-streaming/attachments-streaming-pool.ts

radovanjorgic · 2026-02-19T05:19:26Z

src/multithreading/worker-adapter/worker-adapter.ts

+            error: {
+              message: `Error while preparing artifact for attachment ID ${
+                attachment.id
+              }. Skipping attachment. ${JSON.stringify(


We stringify in serializeError already so we don't need to stringify here, right?

radovanjorgic · 2026-02-19T05:22:56Z

src/multithreading/worker-adapter/worker-adapter.ts

+          return {
+            error: {
+              message:
+                `Error while streaming to artifact for attachment ID ${attachment.id}. Skipping attachment. ` +
+                JSON.stringify(serializeError(uploadedArtifactError)),
+              fileSize: fileSize,
+            },
+          };


Can you check this? I remember we had issues in past when stringifying errors. Were getting empty objects as output sometimes and it was hard to debug.

radovanjorgic · 2026-02-19T05:24:49Z

src/attachments-streaming/attachments-streaming-pool.ts


+        // If response is `undefined`, we have hit the timeout in the adapter.
+        if (!response) {
+          continue;


Why is response undefined if we hit adapter timeout? If there is soft timeout we can still finish this iteretion and next iteration will break from the while loop here.

radovanjorgic · 2026-02-19T05:28:42Z

src/uploader/uploader.ts

    );
    if (fileError) {
      return {
        error: new Error(


Yes but ErrorRecord is type defined by us, while Error is generic error constructor in JavaScript (which coincidentally has message field).

radovanjorgic · 2026-02-19T05:30:44Z

src/uploader/uploader.ts

  ): Promise<UploaderResult<ArtifactToUpload>> {
    const url = `${this.devrevApiEndpoint}/internal/airdrop.artifacts.upload-url`;

+    if (fileSize != null && fileSize! <= 0) {


Okay so we can check it with this only then: if(fileSize! <= 0) {return error}.

Co-authored-by: Radovan Jorgić <radovanjorgic14@gmail.com>

gasperzgonec added 5 commits January 22, 2026 08:15

Added check for Attachment size of 0.

a7bdb3a

Changed error messages for more observability.

8e1fbde

Before, some error messages contained [object Object], so I had to add JSON.stringify(<error>) to fix that. I changed `. "+error` to `.", error`, which automatically adds a space and stringifies the error.

Merge remote-tracking branch 'origin/main' into gasperz/ISS-247801

3771381

Added more information about attachments that fail.

e50ca6b

Fixed backward-compatibility tests.

ab1f3a5

gasperzgonec requested review from a team and radovanjorgic as code owners January 29, 2026 16:29

gasperzgonec changed the title ~~Fix attachment streaming errors and add observability for failed attachments.~~ Fix attachment streaming errors and add observability for failed attachments Jan 29, 2026

radovanjorgic reviewed Jan 29, 2026

View reviewed changes

gasperzgonec added 5 commits January 30, 2026 08:57

Changed so that now every error is printed separately.

7556b49

Merge remote-tracking branch 'origin/main' into gasperz/ISS-247801

b5d449c

Added additional information to the error printing in the upload func…

a1322f9

…tion.

getArtifactUploadUrl no longer returns an error.

5ab53de

Renamed variable and caught the [Object object] prints.

c386d9b

radovanjorgic reviewed Feb 10, 2026

View reviewed changes

gasperzgonec added 4 commits February 12, 2026 12:22

Merge remote-tracking branch 'origin/main' into gasperz/ISS-247801

9afc001

Removed error print on adapter timeout.

846ff30

Changed how serializeError function works.

3f3be30

Ran prettier.

c410cee

radovanjorgic reviewed Feb 19, 2026

View reviewed changes

gasperzgonec and others added 2 commits February 19, 2026 15:32

Update src/attachments-streaming/attachments-streaming-pool.ts

fd492f4

Co-authored-by: Radovan Jorgić <radovanjorgic14@gmail.com>

Merge remote-tracking branch 'origin/main' into gasperz/ISS-247801

c88ba33

Comments

Conversation

gasperzgonec commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Connected Issues

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gasperzgonec Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gasperzgonec commented Jan 29, 2026 •

edited

Loading

gasperzgonec Feb 12, 2026 •

edited

Loading