Repository navigation
feat: reject ref and PdfDocument methods with coded PdfMethodErrors - #73
Conversation
The view's promise methods (getOutline, getPageSizes, renderPage, print, getFormFields, setFormFieldValues, exportDocument) and PdfDocument's reads rejected with plain Errors: no code to match on, messages that differed between platforms, and on Android the exception class and the whole stack trace in the message. They now reject with a PdfMethodError whose code (PdfMethodErrorCode) is the same on both platforms: not-loaded, closed, invalid-argument, not-supported, not-permitted, busy, io-error, invalid-pdf, out-of-memory or unknown. message is one sentence; nativeDescription keeps the platform's text. Nitro rejections carry only a message, so native code rejects with "[code] message" (PdfMethodFailure in Swift, PdfMethodException in Kotlin) and one JS function parses its first line, after Android's exception class (renamed in minified builds) and before the stack trace. Platform errors are coded by type: file system errors are io-error, OutOfMemoryError out-of-memory, parse failures invalid-pdf, and the outline and metadata of encrypted documents on Android not-supported. The Nitro specs are unchanged. The same failure has the same code: a single-choice list box given several options is invalid-argument on iOS too (it was "not supported on iOS"), and Android's renderPage width message no longer reads "4096.0". On iOS, a read or export still waiting for its queue when the view closes or its source changes rejects with closed, as on Android, instead of resolving with the old document's data. The example's event lines show code and message, and the e2e flows match codes instead of message regexes. Behavior change: on Android, error.message no longer holds the exception class and the stack trace (nativeDescription does). Apps that matched on message text should match on code. Closes #65
# Conflicts: # docs/api.md
jkasprzyk17
added
enhancement
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 42.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 36 files. (13 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title uses the required feature prefix and clearly describes the main change. It exceeds the ideal 50-character target at 67 characters, but remains specific and descriptive. Description check The description directly explains the coded PdfMethodError changes, affected APIs, platform behavior, documentation, and testing. Linked Issues check PR #73implements the coding requirements in open issue#65. The seven view methods and fourPdfDocumentreads usePdfMethodErrorwith the separatePdfMethodErrorCodeunion. Swift and Kotlin emi…Out of Scope Changes check The reviewed changes stay within issue #65. Native exception mapping, lifecycle handling, JavaScript error conversion, tests, documentation, example error formatting, and e2e assertion updates directl…Security Check No medium-, high-, or critical-severity vulnerability was introduced. The PR adds error classification and promise wrappers. It does not add network access, authorization changes, deserialization, or … Full details: Linked Issues check
-
Explanation
PR
#73implements the coding requirements in open issue#65. The seven view methods and fourPdfDocumentreads usePdfMethodErrorwith the separatePdfMethodErrorCodeunion. Swift and Kotlin emit coded failures, and JavaScript normalizes them with one parser. The implementation preservesnativeDescription, removes Android stack traces frommessage, aligns platform codes, and handles closed documents.docs/api.mddocuments method codes. The e2e flows assert coded output. JS, JVM, and focused e2e tests cover the requested behavior.
Full details: Out of Scope Changes check
-
Explanation
The reviewed changes stay within issue
#65. Native exception mapping, lifecycle handling, JavaScript error conversion, tests, documentation, example error formatting, and e2e assertion updates directly support coded method errors. The encrypted-document classification and cross-platform form and close behavior also implement the issue's stated same-failure requirements. No unrelated feature or cleanup change is demonstrated.
Full details: Security Check
-
Explanation
No medium-, high-, or critical-severity vulnerability was introduced. The PR adds error classification and promise wrappers. It does not add network access, authorization changes, deserialization, or new file-path behavior. Export path checks remain unchanged on both platforms. Native descriptions retain error text and Android stack traces by design, matching the documented API and existing Android rejection behavior. No prior security findings were supplied.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 42.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 36 files. (13 skipped: 13 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Since gmi-software#73 the ref's promise methods reject with a PdfMethodError, and apps match on its code rather than the message.
* fix: handle null, undefined and malformed sources gracefully (#66) * fix: harden invalid source reporting * fix: clarify invalid source diagnostics * fix: keep overlay context when source is empty * docs: mark source as optional and nullable in api-design * docs: name the not-loaded code for an unmounted PdfView Since #73 the ref's promise methods reject with a PdfMethodError, and apps match on its code rather than the message. --------- Co-authored-by: caja_autoservicio <caja.prat@cademartori.cl> Co-authored-by: Jakub Kasprzyk <kasprzykjakub@proton.me>
1 participant
Closes #65.
What changes
The ref's promise methods (
getOutline,getPageSizes,renderPage,print,getFormFields,setFormFieldValues,exportDocument) andPdfDocument's reads now reject with aPdfMethodError:codeis aPdfMethodErrorCode, the same on both platforms:not-loaded,closed,invalid-argument,not-supported,not-permitted,busy,io-error,invalid-pdf,out-of-memory,unknown.messageis one sentence: no exception class, no stack trace.nativeDescriptionkeeps the platform's text (on Android with the stack trace).PdfMethodErrorCodeis a separate union, so apps that switch exhaustively overonError'sPdfErrorCodedon't break.docs/api.mdlists the codes and when each method rejects with them; the guides name the codes where they describe rejections.How
The issue's option 2: a coded message. The Nitro specs don't change.
[code] message: SwiftPdfMethodFailure, KotlinPdfMethodException.io-error,OutOfMemoryErrorisout-of-memory, parse failures areinvalid-pdf.toPdfMethodError. It reads the first line, after Android's exception class and before the stack trace. The library ships no ProGuard rules, so in minified release builds the class name is renamed, and the parser accepts any class name.unknown.A JS test checks that the Swift and Kotlin code lists match the TS union.
Same failure, same code
invalid-argumenton iOS too, as on Android. A multi-select list box given several options is stillnot-supportedon iOS.getOutlineandgetMetadatafor encrypted documents reject withnot-supported(newEncryptedPdfException), notinvalid-pdf.renderPagewidth message no longer reads4096.0.sourcechanges now rejects withclosed, as on Android. Before, it resolved with the old document's data. Calls already running finish on both platforms.Behavior change (release notes)
On Android,
error.messageno longer holds the exception class and the stack trace;nativeDescriptiondoes. Apps that matched on message text should match oncode.Testing
bun test, 124 tests): the parser (iOS-shaped, Android-shaped with a stack trace, a renamed class, no code, empty, not an Error),not-loadedbefore mount,closedafterclose(), native rejections → codes. Run red before the change.testDebugUnitTest, 299 tests):PdfMethodException's message format, including the first lineprintStackTracewrites (fbjni passes that text to JS), the code of each kind of platform failure, form value codes, andclosedfor work dropped on close.code: message. The flows assert codes instead of message regexes.viewer-sourcesasserts the full one-line message on both platforms.pdf-document,viewer-sources,viewer-forms,viewer-page-image,viewer-outline,viewer-page-sizesandstress-close-during-work, with no crashes.closedon iOS, by hand on both platforms:renderPagecalls at once), then tap "Swap" right away.closed: The document was closed.Not done: the agent skill in better-skills; it follows the release.
Need help on this PR? Tag
@codesmith-botwith what you need.